Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encode control characters in URIUtil.encodePath #6870

Closed
boughtonp opened this issue Sep 19, 2021 · 2 comments · Fixed by #6874 or #6895
Closed

Encode control characters in URIUtil.encodePath #6870

boughtonp opened this issue Sep 19, 2021 · 2 comments · Fixed by #6874 or #6895
Labels
Bug For general bugs on Jetty side

Comments

@boughtonp
Copy link

Jetty version(s)
9.4.43 and 10.0.1

Description
Runing Jetty with DefaultServlet, http://localhost/%0A returns a 404 - as expected.

However, when using BalancerServlet, the same request URL causes a 500 error. Same goes for anything from %01 to %1F (i.e. encoded control characters).

By simplifying to a minimal example, it turns out the issue does not occur without the rewrite module enabled.
More specifically, it still doesn't occur if only a RewriteHandler has been added, but adding a VirtualHostRuleContainer triggers it, whether or not there are rules within that container.

Based on the exception, something is decoding the request URL before it is being passed to a URI.create() call, but given that http://localhost/%20 does NOT trigger the error, it's also being partially re-encoded (otherwise URI.create would complain about the space character, which it doesn't).

So, looking at the code, the bug will most likely be in URIUtil.encodePath - perhaps the (c < 0) should be a (c < 20) or (c < 32 ) ? (either way, looks to me like that function could be cleaned up).

There may also be a secondary issue - is RuleContainer.apply being called when it doesn't need to be?
(if the container is either empty, or the virtualhosts don't match the current request, it should be a no-op?)

How to reproduce?
Bash script demonstrating the issue:

JETTY_HOME=/path/to/jetty-home-9.4.43.v20210629
JETTY_BASE=test-badurl

mkdir -p "$JETTY_BASE"/{webapps,modules,etc}
sed 's!etc/jetty-proxy.xml!#&!' "$JETTY_HOME/modules/proxy.mod" > "$JETTY_BASE/modules/rproxy.mod"

echo '<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure.dtd">
<Configure class="org.eclipse.jetty.servlet.ServletContextHandler">
 <Call name="addServlet">
  <Arg>org.eclipse.jetty.proxy.BalancerServlet</Arg>
  <Arg>/*</Arg>
  <Call name="setInitParameter">
   <Arg>balancerMember.localhost_8123.proxyTo</Arg>
   <Arg>http://localhost:8123/</Arg>
  </Call>
 </Call>
</Configure>' > "$JETTY_BASE/webapps/ROOT.xml"

echo '<?xml version="1.0"?>
<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_0.dtd">
<Configure id="Server" class="org.eclipse.jetty.server.Server">
 <Call name="insertHandler">
  <Arg>
   <New class="org.eclipse.jetty.rewrite.handler.RewriteHandler">
    <Set name="originalPathAttribute">requestedPath</Set>
    <Set name="rewriteRequestURI">true</Set>
    <Set name="rules">
     <Array type="org.eclipse.jetty.rewrite.handler.Rule">
      <Item>
       <New class="org.eclipse.jetty.rewrite.handler.VirtualHostRuleContainer">
       </New>
      </Item>
     </Array>
    </Set>
   </New>
  </Arg>
 </Call>
</Configure>' > "$JETTY_BASE/etc/jetty-rewrite.xml"

cd "$JETTY_BASE"
java -jar $JETTY_HOME/start.jar --add-to-start=http,deploy,rproxy,rewrite

java -jar $JETTY_HOME/start.jar &

curl -isS 'http://localhost:8080/%0A'

Related exception:

2021-09-19 21:31:30.505:WARN:oejs.HttpChannel:qtp5592464-19: /|
java.lang.IllegalArgumentException: Illegal character in path at index 24: http://localhost:8123///|
        at java.net.URI.create(Unknown Source)
        at org.eclipse.jetty.proxy.BalancerServlet.rewriteTarget(BalancerServlet.java:142)
        at org.eclipse.jetty.proxy.ProxyServlet.service(ProxyServlet.java:61)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
        at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799)
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:550)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349)
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
        at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:191)
        at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
        at org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:322)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
        at org.eclipse.jetty.server.Server.handle(Server.java:516)
        at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388)
        at org.eclipse.jetty.server.HttpChannel$$Lambda$87/1776338520.dispatch(Unknown Source)
        at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
        at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
        at java.lang.Thread.run(Unknown Source)
Caused by:
java.net.URISyntaxException: Illegal character in path at index 24: http://localhost:8123///|
        at java.net.URI$Parser.fail(Unknown Source)
        at java.net.URI$Parser.checkChars(Unknown Source)
        at java.net.URI$Parser.parseHierarchical(Unknown Source)
        at java.net.URI$Parser.parse(Unknown Source)
        at java.net.URI.<init>(Unknown Source)
        at java.net.URI.create(Unknown Source)
        at org.eclipse.jetty.proxy.BalancerServlet.rewriteTarget(BalancerServlet.java:142)
        at org.eclipse.jetty.proxy.ProxyServlet.service(ProxyServlet.java:61)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
        at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799)
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:550)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349)
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
        at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:191)
        at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
        at org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:322)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
        at org.eclipse.jetty.server.Server.handle(Server.java:516)
        at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388)
        at org.eclipse.jetty.server.HttpChannel$$Lambda$87/1776338520.dispatch(Unknown Source)
        at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
        at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
        at java.lang.Thread.run(Unknown Source)

/end

@boughtonp boughtonp added the Bug For general bugs on Jetty side label Sep 19, 2021
gregw added a commit that referenced this issue Sep 20, 2021
Reproduce #6870 Rewritten Balancer

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw linked a pull request Sep 20, 2021 that will close this issue
gregw added a commit that referenced this issue Sep 20, 2021
Fix #6870 URIUtil.encodePath encodes control characters

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw changed the title Jetty BalancerServlet responds with 500 Server Error when URL contains encoded control characters. Encode control characters in URIUtil.encodePath Sep 20, 2021
@joakime joakime added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Sep 20, 2021
@joakime joakime added this to To do in Jetty 9.4.44 FROZEN via automation Sep 20, 2021
@joakime joakime moved this from To do to In progress in Jetty 9.4.44 FROZEN Sep 20, 2021
@joakime joakime moved this from To do to In progress in Jetty 10.0.7/11.0.7 FROZEN Sep 20, 2021
gregw added a commit that referenced this issue Sep 21, 2021
Fix #6870 URIUtil.encodePath encodes control characters

* Better test for wider range of characters
* Encode all control characters

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@joakime
Copy link
Contributor

joakime commented Sep 22, 2021

This is merged for 9.4.x
What about 10.0.x?

@joakime joakime moved this from In progress to Done in Jetty 9.4.44 FROZEN Sep 22, 2021
gregw added a commit that referenced this issue Sep 23, 2021
Fix #6870 URIUtil.encodePath encodes control characters

* Better test for wider range of characters
* Encode all control characters

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw
Copy link
Contributor

gregw commented Sep 23, 2021

Doh! created branch but forgot to push....

Jetty 10.0.7/11.0.7 FROZEN automation moved this from In progress to Done Sep 23, 2021
gregw added a commit that referenced this issue Sep 23, 2021
Fix #6870 URIUtil.encodePath encodes control characters

* Better test for wider range of characters
* Encode all control characters

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
3 participants