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

Attribute org.eclipse.jetty.multipartConfig is null #12650

Closed
mperktold opened this issue Dec 18, 2024 · 5 comments · Fixed by #12656
Closed

Attribute org.eclipse.jetty.multipartConfig is null #12650

mperktold opened this issue Dec 18, 2024 · 5 comments · Fixed by #12656
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@mperktold
Copy link

Jetty version(s)
12.0.15

Jetty Environment
ee10

Java version/vendor (use: java -version)
openjdk version "21.0.5" 2024-10-15 LTS
OpenJDK Runtime Environment Temurin-21.0.5+11 (build 21.0.5+11-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.5+11 (build 21.0.5+11-LTS, mixed mode, sharing)

OS type/version
Windows 11

Description
When using a Vaadin starter with Jetty, the logs show the following warnings:

Attribute org.eclipse.jetty.multipartConfig is null

The log message comes from Atmosphere, which is used internally by Vaadin, but to me this seems like a Jetty issue. Atmosphere just cycles through all attributes to copy them into a local field: https://github.com/Atmosphere/atmosphere/blob/46faee5a497174d661e91683af5fad9c4b637f09/modules/cpr/src/main/java/org/atmosphere/cpr/AtmosphereRequestImpl.java#L1441-L1460
It doesn't expect that any attribute name returned by request.getAttributeNames() is mapped to a null value.

And indeed, Jetty only includes this attribute if it there is a non-null value for it, because it uses an implementation of Attributes.Synthetic to add this attribute to the ones of the underlying request:

_attributes = new Attributes.Synthetic(request)
{
@Override
protected Object getSyntheticAttribute(String name)
{
return switch (name)
{
case SSL_CIPHER_SUITE -> super.getAttribute(EndPoint.SslSessionData.ATTRIBUTE) instanceof EndPoint.SslSessionData data ? data.cipherSuite() : null;
case SSL_KEY_SIZE -> super.getAttribute(EndPoint.SslSessionData.ATTRIBUTE) instanceof EndPoint.SslSessionData data ? data.keySize() : null;
case SSL_SESSION_ID -> super.getAttribute(EndPoint.SslSessionData.ATTRIBUTE) instanceof EndPoint.SslSessionData data ? data.sslSessionId() : null;
case PEER_CERTIFICATES -> super.getAttribute(EndPoint.SslSessionData.ATTRIBUTE) instanceof EndPoint.SslSessionData data ? data.peerCertificates() : null;
case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfigElement();
case FormFields.MAX_FIELDS_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormKeys();
case FormFields.MAX_LENGTH_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormContentSize();
default -> null;
};
}
@Override
protected Set<String> getSyntheticNameSet()
{
return ATTRIBUTES;
}
};

The base class should take care of not including the attribute if its value is null.

However, when Atmosphere fetches the values for each attribute name, it ends up in a different code, and in a different instance of ServletHolder, where the value is indeed null:

return _mappedServlet.getServletHolder().getMultipartConfigElement();

How to reproduce?
I've created a repository here: https://github.com/mperktold/hello-vaadin
You can simply start the application and open it in your browser, and you should immediately see the warning in the logs.
The application is a basic Vaadin starter where spring-boot-starter-tomcat is replaced with spring-boot-starter-jetty.

@mperktold mperktold added the Bug For general bugs on Jetty side label Dec 18, 2024
@janbartel
Copy link
Contributor

@mperktold so far I can't seem to replicate this with jetty (without vaadin). I'm checking that after a forward (as indicated by the line number in the Dispatcher), a servlet that enumerates all of the request attributes does not include the org.eclipse.jetty.multipartConfig attribute if the servlet has no multipart configuration.

I downloaded your repo and ran that in IntelliJ, and hit the indicated home url, but I don't see anything referring to the multipart attribute on the output. I'm attaching the output log to this post.
hello-vaadin-main-output.txt

@janbartel janbartel self-assigned this Dec 19, 2024
@janbartel
Copy link
Contributor

A follow up - I've been able to see the log output, but it only happens occasionally. In the debugger, it looks like it could be during the handling of this request:

this.request = {Dispatcher$ForwardRequest@12914} "ForwardRequest@7966935c{ServletApiRequest@7b0fce3f{ServletContextRequest@539f6e94{EventsRequest@3c1bc18{POST@e02a580 http://localhost:8080/VAADIN/push?v-r=push&v-uiId=0&v-pushId=81146069-c041-4452-abc6-7038bddf5963&X-Atmosphere-Transport=close&X-Atmosphere-tracking-id=cdce9031-5d31-4a5f-aea3-02a2dce984c6&X-Vaadin-LastSeenServerSyncId=7 HTTP/1.1}}}}"

janbartel added a commit that referenced this issue Dec 19, 2024
Only return the org.eclipse.jetty.multipartConfig attribute name on
getAttributeNames iff the servlet that is the target of the forward
supports multipart.
@janbartel
Copy link
Contributor

A follow up to the follow up: I think there may be an anomaly with the way we calculate the applicable attribute names, vs the value of the attribute. It could be that we're deciding the multipart attribute name should be included in the nameset based on the value of the MultipartConfigElement of the servlet originating the forward, whereas we return the value of the multipart attribute for the destination servlet (ie on the other side of the forward).

It looks to me that the originating servlet is a org.springframework.web.servlet.DispatcherServlet, and the destination servlet is a com.vaadin.flow.spring.SpringServlet. Does that sound plausible?

Let me confirm that with a bit more investigation, it's very confusing with so much request wrapping and forwarding happening. Standby.

@janbartel
Copy link
Contributor

See PR #12656

janbartel added a commit that referenced this issue Dec 20, 2024
* Issue #12650 Fix multipart attribute for forwards.

Only return the org.eclipse.jetty.multipartConfig attribute name on
getAttributeNames iff the servlet that is the target of the forward
supports multipart.
@mperktold
Copy link
Author

@janbartel Yes, that sounds very plausible.
Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants