[JENKINS-59849] Don't fail to serve resource files with nontrivial names - #4302
Conversation
daniel-beck
added
the
bug
| } | ||
|
|
||
| private static Token decode(String value) { | ||
| byte[] byteValue = Base64.getUrlDecoder().decode(value); |
There was a problem hiding this comment.
Not strictly related, but @jsoref could make this throw an exception, so just move it down.
|
amending #4239 |
| restOfPath = "/" + restOfPath; | ||
| } | ||
| return resourceRootUrl + getUrlName() + "/" + token.encode() + restOfPath; | ||
| return resourceRootUrl + getUrlName() + "/" + token.encode() + Arrays.stream(restOfPath.split("[/]")).map(Util::rawEncode).collect(Collectors.joining("/")); |
There was a problem hiding this comment.
This is going to drop a trailing / so that is no good. It will also convert "/" to "".
There was a problem hiding this comment.
Added a test that hopefully demonstrates that this does not matter.
Redirect URLs are only ever obtained for files, and directory browsing on the resource domain continues to work.
|
Fwiw, I tested this PR using: node('master') {
sh '''A='a/'"$(perl -e 'for ($a=1; $a < 50; $a++) { next if $a >= 10 && $a <= 13; next if $a == 8 || $a == 47; print "$a ".chr($a)." "}')"
mkdir -p "$A"
echo "hello" > "$A/index.html"
'''
archiveArtifacts 'a/'
}And it worked... including browsing directories... |
| public Token getToken(@Nonnull DirectoryBrowserSupport dbs, @Nonnull StaplerRequest req) { | ||
| // This is the "restOfPath" of the DirectoryBrowserSupport, i.e. the directory/file/pattern "inside" the DBS. | ||
| final String dbsFile = req.getRestOfPath(); | ||
| final String dbsFile = req.getOriginalRestOfPath(); |
There was a problem hiding this comment.
This is the most important part of the fix, otherwise the math below will be off: We subtracted the length of foo bar from a string ending in whatever/foo%20bar…
jeffret-b
left a comment
jeffret-b
left a comment
There was a problem hiding this comment.
Looks like it should do the job. I've read through the code but haven't done any additional testing.
|
Manually tested with 𐎘 from #4152 in job name (and URL) as well as file name. |
5 participants
See JENKINS-59849.
Proposed changelog entries
Submitter checklist
* Use the
Internal:prefix if the change has no user-visible impact (API, test frameworks, etc.)