gravitee-io / issues

Gravitee.io - API Platform - Issues
65 stars 26 forks source link

Dynamic Routing Policy is attempting to run regex on a %-decoded path #1307

Open Yaytay opened 6 years ago

Yaytay commented 6 years ago

Expected Behavior

A request of the form: POST /test/option/file/e1fae1a7-64dc-4a5d-ab48-823c191e80ae/12376-6167-1-6-1-1-1/!%C2%A3%24%25%5E%26().docx should work.

Current Behavior

Caused by: java.lang.IllegalArgumentException: invalid hex byte '^&' at index 162 of 'http://test:8080/services/rest/evaluation/fileUpload/e1fae1a7-64dc-4a5d-ab48-823c191e80ae/12376-6167-1-6-1-1-1/!£$%^&().docx'
at io.netty.util.internal.StringUtil.decodeHexByte(StringUtil.java:244)
at io.netty.handler.codec.http.QueryStringDecoder.decodeComponent(QueryStringDecoder.java:351)
at io.netty.handler.codec.http.QueryStringDecoder.path(QueryStringDecoder.java:170)
at io.gravitee.gateway.http.core.invoker.DefaultInvoker.selectUserDefinedEndpoint(DefaultInvoker.java:211)
at io.gravitee.gateway.http.core.invoker.DefaultInvoker.selectEndpoint(DefaultInvoker.java:183)
at io.gravitee.gateway.http.core.invoker.DefaultInvoker.invoke(DefaultInvoker.java:89)
at io.gravitee.gateway.handlers.api.ApiReactorHandler.lambda$handleClientRequest$6(ApiReactorHandler.java:154)
at io.gravitee.gateway.policy.impl.RequestPolicyChainProcessor.execute(RequestPolicyChainProcessor.java:65)
at io.gravitee.gateway.policy.impl.RequestPolicyChainProcessor.lambda$execute$0(RequestPolicyChainProcessor.java:57)
at io.gravitee.gateway.policy.impl.PolicyChain.doNext(PolicyChain.java:76)
at io.gravitee.gateway.policy.impl.StreamablePolicyChain.doNext(StreamablePolicyChain.java:51)
at io.gravitee.policy.dynamicrouting.DynamicRoutingPolicy.onRequest(DynamicRoutingPolicy.java:129)

Possible Solution

I'm not sure when it's happening, but clearly something is %-decoding the path and the Dynamic Routing Policy seems to be causing it to double-decode. For most things like this Dynamic Route it will be necessary to keep the path %-encoded all the way through Gravitee. It is never possible to %-decode a path and keep it as a single string (because a %2F will change the meaning of the path).

Steps to Reproduce (for bugs)

  1. Create an API with a Dynamic Route that has a regex like: /option/file/([^/]+)/([^/]+)/([^/]+)
  2. Test with a URL like /option/file/first/second/!%C2%A3%24%25%5E%26().docx
  3. Test with a URL like /option/file/first/second/file%2Fname.docx Both should match the regex and be routed successfully.

Context

We have to accept filenames specified by end users, and those nasty people can put anything in. We need to ensure that anything that is correctly encoded can make it through Gravitee.

Your Environment

brasseld commented 6 years ago

Hi @Yaytay,

I'm not able to reproduce with 1.17.0.

Can you have a try with this version and if you are able to reproduce, please share the log and an export of your API definition.

Cheers!

Yaytay commented 6 years ago

Hi @brasseld Do you have a release date for 1.17.0? Jim

brasseld commented 6 years ago

We expect the release for today.

Can you give me the target URI you specify in the dynamic routing policy. I think I'm not able to reproduce because I4m not using the same target.

Thanks a lot.

Yaytay commented 6 years ago

The policy config is:

            {
              "pattern": "/option/file/(.*)/(.*)/(.*)",
              "url": "{#endpoints['default']}/fileUpload/{#group[0]}/{#group[1]}/{#group[2]}"
            },

and the endpoints look like this (on the test system there is only one endpoint):

    "endpoints": [
      {
        "name": "default",
        "target": "http://internal.host.name:8080/ServiceName/path1/path2/path3",
        "weight": 1,
        "backup": false,
        "type": "HTTP",
        "http": {
          "connectTimeout": 5000,
          "idleTimeout": 60000,
          "keepAlive": true,
          "readTimeout": 10000,
          "pipelining": false,
          "maxConcurrentConnections": 100,
          "useCompression": true,
          "followRedirects": false
        }
      }
    ],