[improve][proxy] Add regression tests for package upload with 'Expect: 100-continue'

[nodece]

Motivation

This pull request introduces improvements and new tests for package management and proxy upload functionality in Pulsar.

Although PulsarAdmin (based on async-http-client) does not send the 'Expect: 100-continue' header by default, we observed in production that some HTTP clients (for example curl or other generic upload tools) do send this
header when uploading large packages through the Pulsar proxy.

In older Pulsar versions, such requests may lead to Early EOF issues during package uploads through the proxy. Adding coverage for this scenario helps ensure that large uploads using 'Expect: 100-continue' work reliably and
remain compatible with common HTTP clients.

Modifications

Package management improvements

• Refactored MockedPackagesStorage.writeAsync to read input streams using a buffer and ByteArrayOutputStream, improving correctness for large files and for streams where InputStream.available() is unreliable.
• Added MockedPackagesStorageTest to validate correct write and read behavior for large package data in the mocked storage implementation.

Proxy upload tests

• Added a new integration test ProxyPackagesUploadTest to verify package upload and download through the Pulsar proxy and broker.
• The test covers large file uploads and explicitly exercises requests using the 'Expect: 100-continue' header to prevent regressions related to Early EOF issues observed in production.

Codebase simplification

• Simplified AdminProxyHandler by reusing the HTTP client configuration from the superclass and customizing only the required protocol handlers, reducing code duplication and improving maintainability.

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

[Copilot]

Pull request overview

This PR fixes proxy package uploads when clients send Expect: 100-continue by ensuring the proxy does not forward the Expect header to brokers, preventing Early EOF errors during large uploads.

Changes:

• Add request wrapping in AdminProxyHandler to strip the Expect header when Expect: 100-continue is present.
• Add proxy package upload/download tests, including an Expect: 100-continue scenario with a large multipart upload.
• Improve mocked packages storage test implementation to read streams in chunks and add unit tests for large read/write correctness.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java	Wraps requests that use Expect: 100-continue to prevent forwarding the Expect header to brokers.
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyPackagesUploadTest.java	Adds integration coverage for uploading/downloading packages through the proxy, including Expect: 100-continue.
pulsar-proxy/pom.xml	Adds pulsar-package-core test-jar dependency for proxy tests using mock package storage.
pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/MockedPackagesStorage.java	Refactors mocked storage writes to read streams in buffered chunks (supports large uploads).
pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/MockedPackagesStorageTest.java	Adds a unit test validating mocked storage write/read correctness for large payloads.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lhotari]

• Added logic in AdminProxyHandler to detect requests with the
  "Expect: 100-continue" header and wrap them using
  NoExpectRequestWrapper, ensuring the header is not forwarded to the
  broker and preventing Early EOF.
• Introduced the NoExpectRequestWrapper inner class to filter out the
  "Expect" header from incoming requests.

I'm not sure if this is the correct approach. Expect: 100-continue is related to Transfer-Encoding: Chunked. Usually Transfer-Encoding: Chunked is used when Content-Length header isn't set due to the size of the body being unknown when the request is started.

While upgrading Jetty from 9.x to 12.x, I noticed that ProxyContinueProtocolHandler was missing from AdminProxyHandler:

pulsar/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java

Lines 182 to 198 in 6d415c6

	protocolHandlers.put(new ProxyContinueProtocolHandler());
	return client;
	} catch (Exception x) {
	throw new ServletException(x);
	}
	}
	class ProxyContinueProtocolHandler extends ContinueProtocolHandler {
	@Override
	protected Runnable onContinue(Request request) {
	HttpServletRequest clientRequest =
	(HttpServletRequest) request.getAttributes().get(CLIENT_REQUEST_ATTRIBUTE);
	return AdminProxyHandler.this.onContinue(clientRequest, request);
	}
	}

A similar change should be implemented in branch-4.1, branch-4.0 and branch-3.0 to fix the Transfer-Encoding: Chunked and Expect: 100-continue support.

In addition, if httpMaxRequestSize is set in configuration, that would block Transfer-Encoding: Chunked:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/MaxRequestSizeFilter.java

Lines 44 to 66 in 6d415c6

	public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
	throws IOException, ServletException {
	long size = request.getContentLengthLong();
	if (size > maxSize || isChunked(request)) {
	// Size it's either unknown or too large
	HttpServletResponse httpResponse = (HttpServletResponse) response;
	httpResponse.sendError(HttpServletResponse.SC_BAD_REQUEST, "Bad Request");
	} else {
	chain.doFilter(request, response);
	}
	}
	private static boolean isChunked(ServletRequest request) {
	if (request instanceof HttpServletRequest) {
	HttpServletRequest req = (HttpServletRequest) request;
	String encoding = req.getHeader("Transfer-Encoding");
	return encoding != null && encoding.contains("chunked");
	} else {
	return false;
	}
	}

[lhotari]

Expect: 100-continue is related to Transfer-Encoding: Chunked.

I checked this claim that I made. It's not correct.

[lhotari]

• Added logic in AdminProxyHandler to detect requests with the
  "Expect: 100-continue" header and wrap them using
  NoExpectRequestWrapper, ensuring the header is not forwarded to the
  broker and preventing Early EOF.

The reason why this doesn't make sense is that the client won't be sending a body when the request contains Expect: 100-continue. Forwarding such a request to the backend server is not useful.
I think it's better to add the continue support using protocolHandlers.put(new ProxyContinueProtocolHandler()) as mentioned in my previous comment.
An alternative approach would be to send a response with status code 100 (continue) without contacting the backend at all. The client would then send the actual request after that.

[nodece]

Both the jetty 9.x and 12.x have this issue.

[nodece]

Now, the client sent "Expect: 100-continue". Based on curl verbose output, it seems Jetty responds with "100 Continue" automatically.

[nodece]

I will try to use the ProxyContinueProtocolHandler tomorrow.

[lhotari]

I will try to use the ProxyContinueProtocolHandler tomorrow.

One possible approach would be to change the createHttpClient method in org.apache.pulsar.proxy.server.AdminProxyHandler so that it calls super.createHttpClient() and then apply the customization client.getProtocolHandlers().put(new RedirectProtocolHandler(client)). I didn't spot other relevant differences. This type of change would remove the need for code duplication in AdminProxyHandler and would be useful for both Jetty 9.x and 12.x implementations.
The ProxyContinueProtocolHandler isn't visible to AdminProxyHandler and in the Jetty 12.x upgrade PR I had to copy the class to add similar logic. That wouldn't be necessary with the HttpClient client = super.createHttpClient(); client.getProtocolHandlers().put(new RedirectProtocolHandler(client)); return client implementation for createHttpClient in AdminProxyHandler.

[nodece]

Thanks for the suggestion, and I agree that this is a better and cleaner approach.

After validating the behavior with packet captures, I realized that my previous change only guarantees correct handling of Expect: 100-continue on the client → proxy hop. In that setup, Jetty on the proxy side consumes the Expect header and sends the 100 Continue response back to the client, but the proxy → broker request does not necessarily follow the same continue semantics.

Your proposal to have AdminProxyHandler#createHttpClient call super.createHttpClient() and then apply the additional customization (e.g. registering RedirectProtocolHandler) ensures that the standard Jetty ContinueProtocolHandler logic is preserved consistently. This makes the Expect: 100-continue behavior work end-to-end, including the proxy → broker hop, rather than being limited to the incoming request only.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lhotari]

LGTM. Great work @nodece