Description
Issue Description
Parse Server expects application keys, tokens, identifiers, client versions, etc. to all be exactly one key/value pair when retrieved from headers. This is to be expected and should be adhered to. However it is also valid to have multiple headers, all with the same (or different) values, so long as they can be combined into one header. As mentioned in RFC2616 Section 4.2, the particular section mentions:
Multiple message-header fields with the same field-name MAY be
present in a message if and only if the entire field-value for that
header field is defined as a comma-separated list [i.e., #(values)].
It MUST be possible to combine the multiple header fields into one
"field-name: field-value" pair, without changing the semantics of the
message, by appending each subsequent field-value to the first, each
separated by a comma. The order in which header fields with the same
field-name are received is therefore significant to the
interpretation of the combined field value, and thus a proxy MUST NOT
change the order of these field values when a message is forwarded.
A bit of a mouthful, but to skip to the issue in question node conveniently coalesces duplicate headers for us as it stands, adhering to this. Since this is the case, and if you happen to have a funky client that sends duplicate headers, you will end up with one more more values instead of the expected singular value. Effectively this:
X-Parse-Application-Id: my-app-id
X-Parse-Application-Id: my-app-id
is coalesced into this
X-Parse-Application-Id: my-app-id, my-app-id
Which is actually valid, but within middlewares.js headers are expected as singular values like so:
var info = {
appId: req.get('X-Parse-Application-Id'),
sessionToken: req.get('X-Parse-Session-Token'),
masterKey: req.get('X-Parse-Master-Key'),
installationId: req.get('X-Parse-Installation-Id'),
clientKey: req.get('X-Parse-Client-Key'),
javascriptKey: req.get('X-Parse-Javascript-Key'),
dotNetKey: req.get('X-Parse-Windows-Key'),
restAPIKey: req.get('X-Parse-REST-API-Key'),
clientVersion: req.get('X-Parse-Client-Version')
};Given this, any client that submits duplicate headers for any of these expected values will be unable to authenticate, or may provide faulty information.
Modifying the aforementioned code locally to properly check, split and take the first argument if there is more than 1 value present results in expected behavior within the php sdk. Running the existing tests in parse-server also indicate proper behavior as well with this change.
Steps to reproduce
In an attempt to integrate hhvm support for the php sdk I have discovered that recent builds (across platforms) for hhvm end up duplicating custom headers. I have observed this same issue on a local machine (OSX) and upstream in a Linux Trusty box (Travis). Specifically this only happens when utilizing the stream client to perform requests, which is the fallback http client to the primary curl one.
After extensive research it would appear this is a bug in hhvm itself. Although that's an issue on it's own, it's what originally brought the thought of how we should be handling the incidental (or buggy) case of duplicate headers.
Expected Results
Should be taking the 1st (or last, order is important) value present when there is more than one value for a given for an api key/token/client version/etc..
Actual Outcome
Passing all comma separated values as one value, which fails to match for authentication purposes
Environment Setup
- Server
- parse-server version: 2.5.2 (can test on 2.5.3 as needed)
- Operating System: OSX
- localhost
tl;dr (in summary...)
Current parse-server expect headers to each have one value. In reality each header could potentially be a comma-separated list of two or more values. Goofy client that sends legitimate requests with duplicated headers would be coalesced by the server and would fail to validate as a result.
Let me know what your thoughts are. It's not exactly needed to support this as we don't necessarily need to fix just for one extra possible platform, and it's not exactly pretty either. There's also the distinct possibility that if somebody has commas in their keys it could mess things up :/.