[Improvement] The Metadata should handle all the case sensitivities

[sigee]

What would you like to be improved?

According to https://www.ietf.org/rfc/rfc2616.txt (4.2 Message Headers), HTTP headers are case insensitive.

The Metadata should handle all the case sensitivities, not just lowercase.

Example test case:

    @Test
    void testGetFirstValueShouldReturnValueWithoutDependingOnCaseSensitivity() {
        Metadata metadata = new Metadata();
        metadata.addValue("location", "http://example.com");
        Assertions.assertEquals("http://example.com", metadata.getFirstValue("location"));
        Assertions.assertEquals("http://example.com", metadata.getFirstValue("Location"));
        Assertions.assertEquals("http://example.com", metadata.getFirstValue("LOCATION"));

        Metadata metadata2 = new Metadata();
        metadata2.addValue("Location", "http://example.com");
        Assertions.assertEquals("http://example.com", metadata2.getFirstValue("location"));
        Assertions.assertEquals("http://example.com", metadata2.getFirstValue("Location"));
        Assertions.assertEquals("http://example.com", metadata2.getFirstValue("LOCATION"));

        Metadata metadata3 = new Metadata();
        metadata3.addValue("LOCATION", "http://example.com");
        Assertions.assertEquals("http://example.com", metadata3.getFirstValue("location"));
        Assertions.assertEquals("http://example.com", metadata3.getFirstValue("Location"));
        Assertions.assertEquals("http://example.com", metadata3.getFirstValue("LOCATION"));
    }

How should we improve?

No response

[rzo1]

Might be duplicate of #108 especially the spellcheck part of Nutch + a normalization step? :)

[TamimEhsan]

@sigee to implement this behaviour, we can normalize the key to lowercase at the start of each function (addValues, setValues, remove etc). Is this a viable solution or something else?

[sigee]

Well, I'm not 100% sure what would be the best solution here, but I would use equalsIgnoreCase for checking if something is present or not.
My thoughts are to let the keys as they are in case if we use org.apache.http.HttpHeaders constants (E.g.: HttpHeaders.ACCEPT which is "Accept") and for some reasons someone wants to check euality case sensitively.
Maybe I just overthinking. I don't know.

@rzo1 What do you think?

[rzo1]

How does Nutch handle it? Would do it similar (as suggest by Julien in the original issue), perhaps even with the SpellCorrection.

[TamimEhsan]

What I understood after reading the Nutch codebase: might be wrong though.

Nutch has two class SpellCheckedMetadata and CaseInsensitiveMetadata both extending the Metadata class.

CaseInsensitiveMetadata as the name suggests is case insensitive, but is not typo tolerant.

On the other hand, SpellCheckedMetadata works a bit differently. There are some pre fixed common headers. The class tries to normalize keys to these headers with LevenshteinDistance for typo tolerance. However, if the key doesn't match any of the pre fixed headers, then it is kept as it is, ie not case sensitive then.

[sebastian-nagel]

See the discussion in NUTCH-3002 and apache/nutch#777. While the "old" HTTP protocol implementations supporting only HTTP 1.0 and 1.1 rely on SpellCheckedMetadata, protocol-okhttp which (optionally) supports HTTP/2 uses CaseInsensitiveMetadata. With binary headers in HTTP/2, spell-checking header names seems somewhat unnecessary. But matching lower-case names is simply mandatory with HTTP/2.

[sebastian-nagel]

On the other hand, SpellCheckedMetadata works a bit differently. [...] However, if the key doesn't match any of the pre fixed headers, then it is kept as it is, ie not case sensitive then.

Good catch! Of course, the crawler does not really care about other headers than the ones listed in HttpHeaders with well-defined semantics and relevance for the crawling process. I've opened NUTCH-3122 to track this. Thanks for your observation!

[rajucomp]

@rzo1 I would like to take on this issue, but I’d appreciate some clarification on a few observations regarding the current implementation.

Observations

1. The behavior of addValue, getValues, remove, and containsKey is inconsistent.
2. Specifically:
   • getValues and containsKey normalize keys to lowercase in case the original key is not found. It doesn't make sense to do that if we are not doing the same to addValues.
   • addValues and remove do not.
3. This results in inconsistent semantics. All key-based operations should follow the same case-handling strategy.

Prior Art

I reviewed how Apache Nutch approaches this (PR #777). They use a TreeMap with a case-insensitive comparator. While this ensures correctness, it changes the time complexity of operations to O(log N), which may not be ideal from a performance standpoint.

Proposed Options

1. Use CaseInsensitiveMap from Apache Commons Collections:

   • https://commons.apache.org/proper/commons-collections/apidocs/org/apache/commons/collections4/map/CaseInsensitiveMap.html
   • This preserves HashMap-like performance characteristics while enforcing consistent, case-insensitive behavior.
   • This would involve replacing the underlying HashMap with CaseInsensitiveMap.

2. Introduce a dedicated implementation:

   • Extend Metadata and introduce a CaseInsensitiveMetadata class.
   • Override the constructor to initialize a case-insensitive backing map.
   • This avoids changing existing behavior by default while offering a clear alternative.

Open Questions

• Should existing usages (new Metadata()) be migrated to CaseInsensitiveMetadata, or should the behavior of Metadata itself be changed?
• Are there any compatibility concerns or downstream assumptions about case sensitivity that should be preserved?

I’d like to align on the preferred approach before starting work on this issue. Thanks.

[rzo1]

Thanks for the reference. In our case, the number of metadata entries is normally limited (typically well below ~100 keys), so the practical impact of moving from O(1) to O(log N) is negligible, imho. Given such small N, I don’t think time complexity is a meaningful consideration here, and prioritizing correctness (CaseInsensitiveMap will violate some Map contracts) and clarity over (theoretical) performance seems reasonable to me.

From my POV we can change the behaviour of Metadata, but this is open to discussion. If we opt to introduce an additional class CaseInsensitiveMetadata, we need an option to configure it (either programmatically by the implemeting user or via SC's configuration).

WDYT @jnioche @sebastian-nagel @mvolikas @sigee ?

[sigee]

Hi,
It was a while ago when I changed the MetadataFilter and I also noticed the inconsistency in the case sensitivity. This is the whole reason behind this issue. In HTML there is no diffrence between lower or uppercase, so it would be nice if Metadata worked in the case insensitive way in my opinion. the mentioned inconsistency in the getValues/containsKey and the addValues/remove kind of handles it under the hood, but not properly.
You can add any key in any case, but when you check or get the values it does not matter what cases did you add. Unfortunatelly there is some prefferences as you mentioned by checking exactly before checking case insesitivly.
So I think it could be everywhere case insensitive, but I'm not sure what can go wrong because of it. Or how should it be done in an efficient way.
Personaly I do not like the new CaseInsensitiveMetadata idea, because as far as I know, there sould not be anything else, just case insensitive one.