eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.31k stars 2.08k forks source link

queryString key should be case sensitive #2096

Open lakemove opened 7 years ago

lakemove commented 7 years ago

given url /test?key=v1&Key=v2&KEY=v3 it should return different value as below, but now it all returns v3

rc.request.getParam("key") //should return v1
rc.request.getParam("Key") //should return v2
rc.request.getParam("KEY") //should return v3

6.2.2.1. Case Normalization

When a URI uses components of the generic syntax, the component syntax equivalence rules always apply; namely, that the scheme and host are case-insensitive and therefore should be normalized to lowercase. For example, the URI is equivalent to http://www.example.com/.

The other generic syntax components are assumed to be case-sensitive unless specifically defined otherwise by the scheme (see Section 6.2.3).

The code causing issue is HttpUtils.params , it should not use CaseInsensitiveHeaders

  static MultiMap params(String uri) {
    QueryStringDecoder queryStringDecoder = new QueryStringDecoder(uri);
    Map<String, List<String>> prms = queryStringDecoder.parameters();
    MultiMap params = new CaseInsensitiveHeaders();
    if (!prms.isEmpty()) {
      for (Map.Entry<String, List<String>> entry: prms.entrySet()) {
        params.add(entry.getKey(), entry.getValue());
      }
    }
    return params;
  }
vietj commented 7 years ago

yes it should

jsight commented 7 years ago

PR: https://github.com/eclipse/vert.x/pull/2125

I know nothing of Vert.x coding practices, though, so flame away at my mistakes. :)

cescoffier commented 1 year ago

@vietj this topic just popped up in Quarkus.

vietj commented 1 year ago

@cescoffier that would be a breaking change, so I think we can make it as an option for now and make it the default in Vert.x 5