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

Simplified QuotedStringTokenizer #9729

Merged
merged 24 commits into from
May 18, 2023
Merged

Simplified QuotedStringTokenizer #9729

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
16aabb3
Simplified QuotedStringTokenizer
gregw May 4, 2023
1b5b895
Simplified QuotedStringTokenizer
gregw May 4, 2023
0d23c57
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12-simp…
gregw May 5, 2023
a7e9016
Fixed tests
gregw May 6, 2023
e52910e
WIP
gregw May 8, 2023
064e4be
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12-simp…
gregw May 10, 2023
2138f3c
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12-simp…
gregw May 10, 2023
75d0008
introduced builder
gregw May 11, 2023
8cdfc77
Extracted QuotedStringTokenizer interface and re-introduced the legac…
gregw May 11, 2023
0523890
Merge branch 'jetty-12.0.x' into jetty-12-simplify-QuotedStringTokenizer
gregw May 12, 2023
bd86c23
Re-introduced the ability to have unescaped \ in filenames
gregw May 12, 2023
f5abc8c
Fixed tests
gregw May 12, 2023
83bd9c8
Whitespace is Character.isWhiteSpace
gregw May 12, 2023
20bdcbd
Disable test pending RFC8187
gregw May 13, 2023
c019dfd
Merge branch 'jetty-12.0.x' into jetty-12-simplify-QuotedStringTokenizer
gregw May 15, 2023
a5050e6
updates from review
gregw May 15, 2023
fe189bb
updates from review
gregw May 16, 2023
82071f9
updates from review
gregw May 16, 2023
743ad3d
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12-simp…
gregw May 16, 2023
6f6832f
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12-simp…
gregw May 17, 2023
54e1a55
Updates from review
gregw May 17, 2023
03880a8
Updates from review
gregw May 17, 2023
7cec3c9
Updates from review
gregw May 17, 2023
16da20c
No OWS around =
gregw May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,11 +1648,7 @@ private String fileNameValue(String token)
}
else
{
// unquote the string, but allow any backslashes that don't
// form a valid escape sequence to remain as many browsers
// even on *nix systems will not escape a filename containing
// backslashes
return QuotedStringTokenizer.unquoteOnly(value, true);
return QuotedStringTokenizer.unquoteOnly(value);
}
}

Expand Down
5 changes: 1 addition & 4 deletions jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.jetty.io.ArrayByteBufferPool;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.io.RetainableByteBuffer;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.BufferUtil;
Expand Down Expand Up @@ -677,7 +674,7 @@ public void testPartWithBackSlashInFileName() throws Exception

String contents = """
--AaB03x\r
content-disposition: form-data; name="stuff"; filename="Taken on Aug 22 \\ 2012.jpg"\r
content-disposition: form-data; name="stuff"; filename="Taken on Aug 22 \\\\ 2012.jpg"\r
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels weird. Why there are now 2 backslashes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we used to forgive browsers for not properly escaping "" in filenames. Now we are going to insist they quote them correctly.

Could be a bad idea. @joakime can you do some tests on windows to see how "" is treated by current browsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted. But there is no good solution. See comments below.

Content-Type: text/plain\r
\r
stuffaaa\r
Expand Down
2 changes: 1 addition & 1 deletion jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedCSVTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,6 @@ public void testJoin()
assertThat(QuotedCSV.join(Collections.singletonList("hi")), is("hi"));
assertThat(QuotedCSV.join("hi", "ho"), is("hi, ho"));
assertThat(QuotedCSV.join("h i", "h,o"), is("\"h i\", \"h,o\""));
assertThat(QuotedCSV.join("h\"i", "h\to"), is("\"h\\\"i\", \"h\\to\""));
assertThat(QuotedCSV.join("h\"i", "h\to"), is("\"h\\\"i\", \"h\to\""));
}
}
Loading