An omnibus PR for changes needed to support webfunctions #10563

gregw · 2023-09-21T10:59:21Z

Web functions are currently supported with servlets. These changes add/move utility classes to core to better support direct usage of core APIs:

Moved Encoded HttpWriter implementations to jetty-io in core.
Added a ChunkAccumulator

Web functions are currently supported with servlets. These changes add/move utility classes to core to better support direct usage of core APIs

increase usage of Charset in request

…ctions

gregw · 2023-09-22T01:52:01Z

See GoogleCloudPlatform/functions-framework-java#235

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java

jetty-core/jetty-io/src/main/java/module-info.java

sbordet · 2023-09-22T14:49:18Z

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java

+     */
+    public boolean add(Chunk chunk)
+    {
+        if (chunk.hasRemaining())


Not sure about this check, I would add regardless.
The problem is that the caller has to check the return value (because otherwise nobody will release the chunk).
This method is currently used only within a test for hasRemaining() and does not check the return value.

It was really to avoid adding error chunks. I'll test that explicitly instead, although that will stop any optimizations of 1 chunk lists if empty chunks are added to it.

Actually changed to do the retain in the method now, so this makes more sense.

Still an awkward API to use in general, see also comment below.

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java

sbordet · 2023-09-24T17:01:08Z

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java

+    protected final OutputStream _out;
+    protected final ByteArrayOutputStream2 _bytes;


Make these private. They either have getters or only used internally by inner classes.

If I make them private I just have to add protected getters. I don't see the point?

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java

gregw · 2023-09-24T23:01:57Z

@sbordet I've made ResponseWriter explicitly depend on our OutputStreamWriter abstraction, but using the name AbstractOutputStreamWriter was both long and a bit meaningless. I have renamed to WriteThroughWriter to capture what it does. I guess technically that should be WriteThroughOutputStreamWriter but that's a mouthful.

Note I also looked at removing even the temporary buffering in the iso and utf8 versions.... as they could easily just call write(int) on the OutputStream for each converted byte. However, there is locking and state checks and all sorts of stuff going on in HttpOutput#write(int) that I think would be best done once rather than for every byte. Thus I think the temporary buffering is worth it. However, I might try to avoid the char buffer....

gregw · 2023-09-26T01:24:46Z

I have a green local build on GoogleCloudPlatform/functions-framework-java#235, so would be good to have this PR in the next release so I can progress next month.

…ctions

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java

jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java

Do not persist a defaulted charset used in the request.

Do not persist a defaulted charset used in the request. Throw UnsupportedEncodingException from getReader

Do not persist a defaulted charset used in the request. Throw `UnsupportedEncodingException` from `getReader`

Do not persist a defaulted charset used in the request. Throw UnsupportedEncodingException from getReader Improve performance with asciiEqualsIgnoreCase HttpMethod is case-insensitive

An omnibus PR for changes needed to support webfunctions

9efeae5

Web functions are currently supported with servlets. These changes add/move utility classes to core to better support direct usage of core APIs

gregw requested review from sbordet and lachlan-roberts September 21, 2023 10:59

gregw added 2 commits September 22, 2023 08:27

WIP

b27de91

increase usage of Charset in request

Merge branch 'jetty-12.0.x' into experiment/jetty-12/updatesForWebFun…

61cd37f

…ctions

gregw mentioned this pull request Sep 22, 2023

Port to use Jetty-12 core without servlets GoogleCloudPlatform/functions-framework-java#235

Draft

Updates

69e8276

sbordet requested changes Sep 22, 2023

View reviewed changes

Updates from review

e96029d

sbordet requested changes Sep 24, 2023

View reviewed changes

gregw added 2 commits September 25, 2023 08:09

Updates from review

50bcf06

Updates from review

1183e7c

gregw added 3 commits September 25, 2023 10:13

Updates from review

23efd4f

Updates from review

b5f0699

WIP

aaceef0

gregw marked this pull request as ready for review September 26, 2023 01:23

gregw requested a review from sbordet September 26, 2023 02:10

gregw added 2 commits September 26, 2023 18:17

Merge branch 'jetty-12.0.x' into experiment/jetty-12/updatesForWebFun…

f414d81

…ctions

Added flush mechanism

676799c

sbordet requested changes Sep 26, 2023

View reviewed changes

gregw added 2 commits September 27, 2023 06:37

Updates from review

5910205

Updates from review

25be1ca

gregw requested a review from sbordet September 26, 2023 20:43

Updates from review

b02e683

sbordet approved these changes Sep 26, 2023

View reviewed changes

Updates from review

870ed72

sbordet approved these changes Sep 26, 2023

View reviewed changes

Updates from review

27ebc12

gregw merged commit 1a207db into jetty-12.0.x Sep 26, 2023

gregw deleted the experiment/jetty-12/updatesForWebFunctions branch September 26, 2023 23:29

gregw added a commit that referenced this pull request Oct 2, 2023

Fix for issue with #10563

a4a91c1

Do not persist a defaulted charset used in the request.

gregw added a commit that referenced this pull request Oct 3, 2023

Fix for issue with #10563

8e36a20

Do not persist a defaulted charset used in the request. Throw UnsupportedEncodingException from getReader

gregw added a commit that referenced this pull request Oct 3, 2023

Fix getCharacterEncoding issue with #10563 (#10650)

8331809

Do not persist a defaulted charset used in the request. Throw `UnsupportedEncodingException` from `getReader`

gregw added a commit that referenced this pull request Oct 3, 2023

Fix for issue with #10563

03c4b6b

Do not persist a defaulted charset used in the request. Throw UnsupportedEncodingException from getReader Improve performance with asciiEqualsIgnoreCase HttpMethod is case-insensitive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

An omnibus PR for changes needed to support webfunctions #10563

An omnibus PR for changes needed to support webfunctions #10563

gregw commented Sep 21, 2023

gregw commented Sep 22, 2023

sbordet Sep 22, 2023

gregw Sep 24, 2023 •

edited

Loading

gregw Sep 24, 2023

sbordet Sep 24, 2023

sbordet Sep 24, 2023

gregw Sep 24, 2023

gregw commented Sep 24, 2023

gregw commented Sep 26, 2023

		protected final OutputStream _out;
		protected final ByteArrayOutputStream2 _bytes;

An omnibus PR for changes needed to support webfunctions #10563

An omnibus PR for changes needed to support webfunctions #10563

Conversation

gregw commented Sep 21, 2023

gregw commented Sep 22, 2023

sbordet Sep 22, 2023

Choose a reason for hiding this comment

gregw Sep 24, 2023 • edited Loading

Choose a reason for hiding this comment

gregw Sep 24, 2023

Choose a reason for hiding this comment

sbordet Sep 24, 2023

Choose a reason for hiding this comment

sbordet Sep 24, 2023

Choose a reason for hiding this comment

gregw Sep 24, 2023

Choose a reason for hiding this comment

gregw commented Sep 24, 2023

gregw commented Sep 26, 2023

gregw Sep 24, 2023 •

edited

Loading