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.

Sign up for GitHub

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

Jump to bottom

Fix #4275 fail URIs with ambiguous segments #5954

Merged
merged 12 commits into from Feb 16, 2021
Merged

Fix #4275 fail URIs with ambiguous segments #5954

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
372b407
Fix #4275 fail URIs with ambiguous segments
gregw Feb 8, 2021
ff64544
updates from review
gregw Feb 11, 2021
31bb297
Merge remote-tracking branch 'origin/jetty-9.4.x' into jetty-9.4.x-42…
gregw Feb 11, 2021
fa9f6c9
updates from review
gregw Feb 11, 2021
07f6123
updates from review of RFC
gregw Feb 12, 2021
b65a739
updates from review of RFC
gregw Feb 14, 2021
7a649f3
Fix test
gregw Feb 14, 2021
89869fa
updates from review of RFC
gregw Feb 15, 2021
db2ee41
Updates from review
gregw Feb 15, 2021
5837772
Updates from review
gregw Feb 15, 2021
1b2414b
updates from review
gregw Feb 16, 2021
4a670fc
updates from review
gregw Feb 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 18 additions & 13 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Objects;

import org.eclipse.jetty.util.ArrayTrie;
import org.eclipse.jetty.util.MultiMap;
Expand Down Expand Up @@ -134,10 +135,14 @@ public HttpURI(String scheme, String host, int port, String path, String param,
_scheme = scheme;
_host = host;
_port = port;
parse(State.PATH, path, 0, path.length());
_param = param;
_query = query;
_fragment = fragment;
if (path != null)
parse(State.PATH, path, 0, path.length());
if (param != null)
_param = param;
if (query != null)
_query = query;
if (fragment != null)
_fragment = fragment;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do null checks for just assignments.
Path needs null check because path.length() may NPE.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing null checks because if we parse the path, we may discover params, fragments and queries in it. So either we through if they are non null, or just not overwrite them unless we have explicit non null values.

}

public HttpURI(HttpURI uri)
Expand Down Expand Up @@ -548,6 +553,7 @@ else if (c == '/')
{
_fragment = uri.substring(mark, end);
i = end;
break;
}
default:
break;
Expand Down Expand Up @@ -609,12 +615,6 @@ else if (_path != null)
}
}

private void decodePath()
{

}


/**
* Check for ambiguous path segments.
*
Expand Down Expand Up @@ -685,10 +685,14 @@ public String getParam()

public void setParam(String param)
{
_param = param;
if (_path != null && !_path.contains(_param))
if (!Objects.equals(_param,param))
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
_path += ";" + _param;
if (_param != null && _path.endsWith(";" + _param))
_path = _path.substring(0, _path.length() - 1 - _param.length());
_param = param;
if (_param != null)
_path = (_path == null ? "" : _path) + ";" + _param;
_uri = null;
}
}

Expand Down Expand Up @@ -824,6 +828,7 @@ public void setPathQuery(String path)
_decodedPath = null;
_param = null;
_fragment = null;
_query = null;
if (path != null)
parse(State.PATH, path, 0, path.length());
}
Expand Down
59 changes: 59 additions & 0 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
View file
Open in desktop
Expand Up @@ -218,6 +218,65 @@ public void testSchemeAndOrAuthority() throws Exception
assertEquals("http:/path/info", uri.toString());
}

@Test
public void testSetters() throws Exception
{
HttpURI uri = new HttpURI();
assertEquals("", uri.toString());

uri = new HttpURI(null, null, 0, null, null, null, null);
assertEquals("", uri.toString());

uri.setPath("/path/info");
assertEquals("/path/info", uri.toString());

uri.setAuthority("host", 8080);
assertEquals("//host:8080/path/info", uri.toString());

uri.setParam("param");
assertEquals("//host:8080/path/info;param", uri.toString());

uri.setQuery("a=b");
assertEquals("//host:8080/path/info;param?a=b", uri.toString());

uri.setScheme("http");
assertEquals("http://host:8080/path/info;param?a=b", uri.toString());

uri.setPathQuery("/other;xxx/path;ppp?query");
assertEquals("http://host:8080/other;xxx/path;ppp?query", uri.toString());

assertThat(uri.getScheme(), is("http"));
assertThat(uri.getAuthority(), is("host:8080"));
assertThat(uri.getHost(), is("host"));
assertThat(uri.getPort(), is(8080));
assertThat(uri.getPath(), is("/other;xxx/path;ppp"));
assertThat(uri.getDecodedPath(), is("/other/path"));
assertThat(uri.getParam(), is("ppp"));
assertThat(uri.getQuery(), is("query"));
assertThat(uri.getPathQuery(), is("/other;xxx/path;ppp?query"));

uri.setPathQuery(null);
assertEquals("http://host:8080", uri.toString());

uri.setPathQuery("/other;xxx/path;ppp?query");
assertEquals("http://host:8080/other;xxx/path;ppp?query", uri.toString());

uri.setScheme(null);
assertEquals("//host:8080/other;xxx/path;ppp?query", uri.toString());

uri.setAuthority(null,-1);
assertEquals("/other;xxx/path;ppp?query", uri.toString());

uri.setParam(null);
assertEquals("/other;xxx/path?query", uri.toString());

uri.setQuery(null);
assertEquals("/other;xxx/path", uri.toString());

uri.setPath(null);
assertEquals("", uri.toString());
}

public static Stream<Arguments> decodePathTests()
{
return Arrays.stream(new Object[][]
Expand Down