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.

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

HttpUrlConnector and JettyConnector doesn't log the headers modified by MultiPartWriter #5529

Open
chkpnt opened this issue Feb 9, 2024 · 2 comments

Comments

@chkpnt
Copy link

chkpnt commented Feb 9, 2024

When using the LoggingFeature, the actual request sent should be logged by the LoggingInterceptor, even if the MultiPartFeature is used. If the headers don't already include a header MIME-Version and a header Content-Type with a boundary, they are added by the MultiPartWriter:

if (entity.getParent() == null) {
final Object value = headers.getFirst("MIME-Version");
if (value == null) {
headers.putSingle("MIME-Version", "1.0");
}
}
// Initialize local variables we need.
final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, MessageUtils.getCharset(mediaType)));
// Determine the boundary string to be used, creating one if needed.
final MediaType boundaryMediaType = Boundary.addBoundary(mediaType);
if (boundaryMediaType != mediaType) {
headers.putSingle(HttpHeaders.CONTENT_TYPE, boundaryMediaType.toString());
}

Unfortunately, the modified headers aren't logged:

Feb. 09, 2024 4:31:44 PM org.glassfish.jersey.logging.LoggingInterceptor log
INFORMATION: 1 * Sending client request on thread Test worker
1 > POST https://example.com/api/document
1 > Accept: application/json
1 > Content-Type: multipart/form-data
1 > User-Agent: jersey-client/3.1.5 (java)
--Boundary_1_905404580_1707400346199
Content-Type: application/octet-stream
Content-Disposition: form-data; filename="file-1065460366320764802.tmp"; size=7110; name="contents"

%PDF-1.3
%���������
...

--Boundary_1_905404580_1707400346199
Content-Type: application/json
Content-Disposition: form-data; name="metadaten"

{"name":"Passport.pdf","type":"Passport"}
--Boundary_1_905404580_1707400346199--

This can lead to time-consuming debugging sessions, as it requires third-party tools to be used as a workaround to verify the actual requests.

It seems LoggingInterceptor is called to early.

@chkpnt
Copy link
Author

chkpnt commented Feb 9, 2024

Notwithstanding the above, HttpUrlConnector doesn't call HeaderUtils.checkHeaderChanges(...) as other connectors. I haven't checked all others, but at least ApacheConnector, Apache5Connector and JettyConnector are calling the header checker.

@jansupol
Copy link
Contributor

jansupol commented Feb 9, 2024

I am not sure the LoggingInterceptor can be modified - it is the chicken-egg issue.

The WriterInterceptor must be invoked before the MessageBodyWriter because it can modify the HttpHeaders. So can the MessageBodyWriter modify the headers and it expects the headers already to be altered by all the Interceptors. So the WritingInterceptor cannot be invoked after the MessageBodyWriter.

As for the header check, the warning is there for the 3rd part connectors so that the modifications to headers, that are not sent to the server are noticed by the users. The ability to send the headers altered by the MBWs is there just recently, For the default HttpUrlConnector, the altered headers were always possible to send and the check was not required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants