coheigea commented on PR #3596:
URL: https://github.com/apache/cxf/pull/3596#issuecomment-6267821070
Problems that remain
1. buildFromEncoded() and toTemplate() corrupt the host. These are the
failing rows above: the host becomes foo.comhttp. The new branch reaches an
older bug in buildUriString. fromPath("http://foo.com";) sets originalPathEmpty
= true, and that flag stops a / being added before a path that doesn’t already
start with one. build() adds the / in buildURI; buildFromEncoded() and
toTemplate() don’t.
2. The same older bug lets path input change the host, and it is already on
main. On both main and this PR,
UriBuilder.fromPath("http://foo.com";).path(".evil.com").buildFromEncoded()
gives http://foo.com.evil.com. Calling build() on the same builder gives the
correct http://foo.com/.evil.com. The trigger is limited to builders started
with fromPath(absoluteUri); fromUri/WebClient bases don’t set the flag. It’s
still path input rewriting the host, which is the class of bug CXF-7986 is
about. A likely fix: in buildUriString, skip the / only when originalPathEmpty
is set and the path starts with ;, which is what the TCK’s empty-path
matrix-parameter case needs. That would also fix item 1. This was a security
problem on main before this PR, so you may want to handle it through the
private security process rather than in a public PR comment.
3. The new tests are too weak to catch this.
testBuildAndSetFullPathWhenUriIsSet and testReplaceFullPathWhenUriIsSet only
check startsWith("http://foo.com";), which http://foo.comhttp:/bar.com/ also
passes. They should check the exact string or getHost(), and cover
buildFromEncoded() too. The WebClient tests have the same weakness:
preservesPreviouslyAddedPathParameters only checks a prefix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
