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

MimeMappings does not include application/wasm #30885

Closed
ememisya opened this issue May 6, 2022 · 9 comments
Closed

MimeMappings does not include application/wasm #30885

ememisya opened this issue May 6, 2022 · 9 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@ememisya
Copy link

ememisya commented May 6, 2022

Please prepend:

mappings.add("wasm", "application/wasm");

To the indicated line. Currently I'm handling recognition of the wasm payload as such:

@Component
public class ServletCustomizer implements EmbeddedServletContainerCustomizer
{

  @Override
  public void customize(final ConfigurableEmbeddedServletContainer container)
  {
    final MimeMappings mappings = new MimeMappings(MimeMappings.DEFAULT);
    mappings.add("wasm", "application/wasm");
    container.setMimeMappings(mappings);
  }
}

Thank you for your time!

See (https://webassembly.org/docs/web/) for further details on the type.

This compilation can be performed in the background and in a streaming manner. If the Response is not CORS-same-origin, does not represent an ok status, or does not match the application/wasm MIME type, the returned promise will be rejected with a TypeError;

This was previously held up by IANA status which has now been resolved.

References:

WebAssembly/spec#573 (comment)

https://www.iana.org/assignments/media-types/application/wasm

#17254

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 6, 2022
@bclozel
Copy link
Member

bclozel commented May 6, 2022

Looking at #17254 (comment), it sounds like we were not waiting for IANA official registration but rather its adoption by Servlet containers. It looks like this MIME type got added in Tomcat with apache/tomcat@54898ee .

Maybe we should consider it then?

@bclozel bclozel added the for: team-attention An issue we'd like other members of the team to review label May 6, 2022
@wilkinsona
Copy link
Member

Tomcat added it in 9.0.37 so I think we should add it. Like Phil, I thought we had some tests that checked our mappings were in sync with Tomcat's but we haven't had a test failure so I'm not sure what's going on there.

@wilkinsona
Copy link
Member

wilkinsona commented May 6, 2022

I think this may explain it:

Both Jetty and Undertow have default mime mappings that Boot's mime mappings are then added to, whereas Tomcat just uses Boot's mime mappings.

So if Jetty or Undertow add new mappings that aren't in Boot's mappings, we'll get a test failure. If Tomcat adds a new mapping, it'll get overwritten by Boot's mappings and we won't notice it.

I think we should try to improve the tests (it's AbstractServletWebServerFactoryTests.mimeMappingsAreCorrectlyConfigured() that covers this at the moment) and add any mappings that are missing, including application/wasm.

@wilkinsona wilkinsona changed the title Add "application/wasm" to MimeMappings MimeMappings does not include all of Tomcat's default mappings May 6, 2022
@wilkinsona wilkinsona added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels May 6, 2022
@wilkinsona wilkinsona added this to the 2.5.x milestone May 6, 2022
@wilkinsona
Copy link
Member

In early 2020, Tomcat aligned the mappings that it uses for embedded instances with those that were configured in its conf/web.xml. As a result of that alignment, we're now missing 837(!) mappings. We've only got 176 mappings at the moment so aligning will result in a significant increase (almost 5x) in the default mappings.

@StefanBratanov
Copy link
Contributor

Hi, is it possible to assign this to me? I can give it a go. 🙂

@StefanBratanov
Copy link
Contributor

StefanBratanov commented May 8, 2022

I gave it a try and raised a PR #30897 . Hope it's not a problem.

I basically added the missing Tomcat default mappings to MimeMappings.java class and modified the test to make it more robust for the future.

I only added the missing mappings and didn't touch the ones who already existed. Though, I noticed some differences in the Tomcat mappings to the already existing Boot mappings. Few differences for example are:

  • Boot: mappings.add("ms", "application/x-wais-source"); Tomcat: ms=text/troff
  • Boot: mappings.add("otf", "application/x-font-opentype"); Tomcat: otf=font/otf

Not sure if this is a concern.

@wilkinsona
Copy link
Member

Thanks, @StefanBratanov, but as indicated by the comments above, I am already working on this.

@wilkinsona wilkinsona self-assigned this May 8, 2022
@StefanBratanov
Copy link
Contributor

Thanks, @StefanBratanov, but as indicated by the comments above, I am already working on this.

Sure, no worries! Thought it's unassigned.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label May 13, 2022
@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.6.x May 19, 2022
@wilkinsona
Copy link
Member

We discussed this today and decided to use this issue to only add a mapping for application/wasm. We've opened #31171 to track broader alignment.

@wilkinsona wilkinsona changed the title MimeMappings does not include all of Tomcat's default mappings MimeMappings does not include application/wasm May 25, 2022
@wilkinsona wilkinsona removed the for: team-meeting An issue we'd like to discuss as a team to make progress label May 25, 2022
@wilkinsona wilkinsona modified the milestones: 2.6.x, 2.6.9 May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants