Bug - Fixed HTTP request headers are case-insensitive

[fredg1]

title should be straightforward.

 

[philmasterplus]

Since fredg1 has kindly provided a patch on my behalf, allow me to elaborate:

KoLmafia's relay browser acts as a proxy between the browser and KoL's servers. It intercepts and parses HTTP requests (e.g. page visits), and depending on the resource being fetched, either passes it along to KoL or executes a relay script (ASH or JS). For this to work properly, KoLmafia must parse the HTTP request headers for vital information, such as the size of the request body.

A HTTP request can contain an arbitrary number of headers. Each header consists of a header name + colon character (:) + value. An example is shown here:

Host: localhost:60080
User-Agent: 
Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.114 Safari/537.36
Accept: */*
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
Referer: http://localhost:60080/relay_my-relay-script.js?relay=true
Content-Length: 57
<...snip...>

Currently (r20685), KoLmafia parses these headers manually (in RelayAgent.java). It does so by checking if each header line starts with any of the header names that KoLmafia cares about (e.g. Content-Length, which is the total size of the request body).

The problem: Although HTTP header names are case-insensitive,[1] KoLmafia does not take this into account when comparing them. Because of this, KoLmafia fails to recognize a header name if has different casing than what it expects.

This is usually not a problem because major web browsers capitalize each word in the header name (e.g. User-Agent). It becomes a problem if the request comes from another tool that does not follow such conventions. For example, webpack-dev-server (used when developing JS relay scripts) transforms header names in all lowercase (e.g. user-agent). While such headers are perfectly valid in HTTP, KoLmafia fails to recognize them. This results in problems when testing relay scripts:

1. A relay script presents a form to the user
2. The user submits the form
3. The browser initiates a HTTP POST request with the form data in the request body, and a Content-Length header describing the size of the data
4. webpack-dev-server intercepts the request, transforms the header names to lowercase, then passes it to KoLmafia
5. KoLmafia attempts to parse the request, but does not recognize content-length. Because of this, it believes that the size of the data is 0, and ignores the request body.
6. KoLmafia executes the relay script
7. The relay script calls form_fields(). Since the request body has been ignored, the form data has been lost, and the function returns an empty map.

While this use case is somewhat niche, it poses a problem to script developers. Webpack-dev-server is a popular tool in the JavaScript world and not supporting it would negatively affect the developer experience. Furthermore, any other tool that lowercases (or uppercases) the HTTP header names would not play well with KoLmafia. Since KoLmafia is relying on an assumption that is not guaranteed by the HTTP spec, the onus is on KoLmafia to improve its conformance.

Fredg1's patch fixes the issue by lowercasing the header before making the comparison. I verified that it fixes the problem with webpack-dev-server.

(Incidentally, the patch fixes another issue. Previously, KoLmafia did not correctly handle whitespace before or after the header values. The patch amends this by calling trim() on each value.)

---

[1] See Section 4.2 of RFC 2616: Hypertext Transfer Protocol -- HTTP/1.1:

Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive.

 

[MCroft]

I re-wrote that as a case statement, because the original (unpatched) series of "if" statements made my teeth itchy. Worked for me, but since the original did, too I'd also like someone to take a look at this patch.

Edit : fred pointed out some style issues and while I have no idea why IJ didn't help me more, the blame is mine. V2 patch here.

 

[philmasterplus]

Your patch works well; HTTP request headers are being parsed correctly. Good job!

 

[MCroft]

Merged into 20689