fix: add correct mimetype for xsl and xslt, fixes #5457 #5458

apotek · 2023-10-25T00:13:40Z

Add correct mimetype for xsl and xslt

Resolves: #5457

The Issue

XSL and XSLT files are being served by nginx with the default mimetype (application/octet-stream instead of the standard application/xslt+xml.

This means that the content-type header is sending the wrong application type to the XSLT parser/transformer application and it therefore fails to process.

How This PR Solves The Issue

This code change overrides the shipping debian /etc/nginx/mime.types with our own ddev-managed /etc/nginx/mime.types file so we can more easily make fixes to the mime types while waiting for the upstream distro to be updated. adds a types{} section to the nginx.conf just under the include mime.types directive where we can add custom mimetypes for ddev.

Manual Testing Instructions

Build your ddev instance
Put an XSL file such as /test.xsl in the document root.
curl -is "https://yoursite.ddev.site/test.xsl" | grep content-type
You should see content-type: application/xslt+xml in the output.

Automated Testing Overview

No tests added. Is there a framework for scripting a test for this in ddev? I read through the testing docs but that all surrounds tests for ddev itself. Please let me know if there is a recommended way I can create a test for this.

Release/Deployment Notes

This does not affect the behavior of other parts of ddev itself.

If a user's application relies on a non-standards-compliant content-type mimetype for .xsl / .xslt files, their application may fail, as we are now serving .xsl and .xslt files according to W3 standards.

This merge request sponsored by Chromatic.

rfay · 2023-10-25T03:26:05Z

This will still need a custom image tag pushed, we'll work on that.

Were you able to manually test it by changing /etc/nginx/mime.types?

apotek · 2023-10-25T06:14:59Z

Were you able to manually test it by changing /etc/nginx/mime.types?

After starting my ddev test project, I executed ddev ssh -s web and manually edited /etc/nginx/nginx.conf to add exactly what the latest commit does, viz:

include mime.types
+ types {
+   application/xslt+xml xsl xslt;
+ }

After kicking nginx, I get this:

curl -i "https://mysite.ddev.site/testxsl.xsl"
HTTP/2 200
accept-ranges: bytes
content-type: application/xslt+xml
...

Which is what we wanted.

I then made sure pre-existing mime.types weren't affected by requesting a few other files:

curl -is "https://mysite.ddev.site/composer.json" | grep content-type
content-type: application/json

curl -is "https://drupal.ddev.site/index.php" | grep content-type
content-type: text/html; charset=UTF-8

curl -is "https://drupal.ddev.site/testcss.css" | grep content-type
content-type: text/css

rfay · 2023-10-25T12:12:03Z

Thank you!

rfay · 2023-10-25T12:49:17Z

I'm quite sure we'll be successful getting this into DDEV core, perhaps a point release in November. I should note that in the meantime you can just use a .ddev/web-build/Dockerfile to add the file. Just put the mime.types in .ddev/web-build and I think this would work for the Dockerfile:

ADD mime.types /etc/nginx

Thanks for your thoughtful approach to testing solving this, and for the PR! Are you going to try to get it in upstream?

apotek · 2023-10-27T02:22:43Z

I'm quite sure we'll be successful getting this into DDEV core, perhaps a point release in November. I should note that in the meantime you can just use a .ddev/web-build/Dockerfile to add the file. Just put the mime.types in .ddev/web-build and I think this would work for the Dockerfile:

Thank you. That's a lot simpler than what I was doing 😓

Thanks for your thoughtful approach to testing solving this, and for the PR! Are you going to try to get it in upstream?

https://trac.nginx.org/nginx/ticket/2552
https://salsa.debian.org/nginx-team/nginx/-/merge_requests/78

rfay · 2023-10-27T02:26:31Z

Impressive!

Resolves: ddev#5457

This reverts commit 098d8cb.

Don't override the shipping mime.types or we will need to maintain the whole file. Better to just add what is needed to the http config Resolves: ddev#5457

rfay · 2023-10-30T22:55:22Z

Pushed a tagged image for this feature, updated the tag DDEV uses, and rebased.

Please test manually @apotek , thanks!

Artifacts at #5458 (comment)

github-actions · 2023-10-30T22:59:42Z

Download the artifacts for this pull request:

See Testing a PR

rfay · 2023-10-30T23:40:10Z

It looks to me like it's working:

$ curl -I https://d10.ddev.site/test.xsl
HTTP/2 200
accept-ranges: bytes
content-type: application/xslt+xml
date: Mon, 30 Oct 2023 23:39:39 GMT
etag: "65403e0e-29c"
last-modified: Mon, 30 Oct 2023 23:36:46 GMT
server: nginx
content-length: 668

rfay

Looks ok to me, tests ok, will look forward to your manual test results. Make sure to remove any custom Dockerfile that you may be using...

apotek · 2023-10-31T20:58:49Z

Please test manually @apotek , thanks!

Thank you @rfay . I read the docs and honestly am unsure of what to do in order to test this. Since this PR does not modify ddev itself, it doesn't seem I would need to use the custom binaries. What I need is to build the nginx docker service image from my merge request's source directory (which I guess would be running make in my ddev repo directory):

exporting to image                                                     3.7s
 => => exporting layers                                                    3.6s
 => => writing image sha256:159dd7d30d479ea6096afbf587d64bf1c39a876eacf3e  0.0s
 => => naming to docker.io/ddev/ddev-webserver-prod:v1.22.4-alpha1-18-g23  0.0s

and then, be able to use ddev-webserver-prod:v1.22.4-alpha1-18-g23 when I run ddev start. That's the part I am unsure of.

Can I somehow change

webserver_type: nginx-fpm

to point to my locally generated nginx-fpm container?

I apologize if this should be obvious to me or if I am missing something.

I did read https://ddev.readthedocs.io/en/latest/developers/building-contributing/#making-changes-to-ddev-images and https://ddev.readthedocs.io/en/latest/developers/building-contributing/#making-changes-to-ddev-images several times. I don't think I would want to push my built image, since I don't want to publish it either to the ddev docker account or my own. I just want to use the built image locally. So that's where I am getting confused.

rfay · 2023-10-31T21:25:14Z

All you have to do is download the ddev artifacts from this PR, you don't have to build anything. And use them. Unzip ddev, follow the instructions, put it in your path or explicitly path it. Then use it in your project. If that doesn't make any sense I'll be happy to do a screenshare call with you, come on over to Discord or whatever, https://discord.gg/hCZFfAMc5k

There is nothing you have to do with DDEV images. I pushed the changed image for you. All you need to use is the updated DDEV in this PR, #5458 (comment) - It will pull the image that I pushed that has your changes in it.

But when you test, make sure you're testing without the Dockerfile you may have added in .ddev/web-build/Dockerfile, which would make your testing invalid.

apotek · 2023-10-31T21:44:12Z

There is nothing you have to do with DDEV images. I pushed the changed image for you. All you need to use is the updated DDEV in this PR, #5458 (comment) - It will pull the image that I pushed that has your changes in it.

Aha! So the binary is modified to use a different tag/image name that you have already pushed up! Now it all makes sense. Thank you! I was getting auth issues on push (for obvious reasons).

My test:

chmod +x ddev    
xattr -r -d com.apple.quarantine ddev  
./ddev start   
...
[build/pull/start output]
...
curl -i https://drupal.ddev.site/testxsl.xsl
HTTP/2 200 
accept-ranges: bytes
content-type: application/xslt+xml

Success :)

rfay · 2023-10-31T22:42:10Z

Thanks for chasing this down! And the upstream issues were a special treat.

apotek requested a review from a team as a code owner October 25, 2023 00:13

github-actions bot added the bugfix label Oct 25, 2023

apotek and others added 4 commits October 30, 2023 16:40

Add correct mimetype for xsl and xslt

8ac6f13

Resolves: ddev#5457

Revert "Add correct mimetype for xsl and xslt"

50598ba

This reverts commit 098d8cb.

Add correct mimetype for xsl and xslt

34d6140

Don't override the shipping mime.types or we will need to maintain the whole file. Better to just add what is needed to the http config Resolves: ddev#5457

Bump image tag for pushed image

234d8c6

rfay force-pushed the 5457-incorrect-mimetype-for-xsl branch from 60aaa98 to 234d8c6 Compare October 30, 2023 22:53

github-actions bot added the dependencies Pull requests that update a dependency file label Oct 30, 2023

rfay approved these changes Oct 30, 2023

View reviewed changes

rfay merged commit d8ebc7d into ddev:master Oct 31, 2023
19 checks passed

apotek deleted the 5457-incorrect-mimetype-for-xsl branch November 1, 2023 03:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add correct mimetype for xsl and xslt, fixes #5457 #5458

fix: add correct mimetype for xsl and xslt, fixes #5457 #5458

apotek commented Oct 25, 2023 •

edited

rfay commented Oct 25, 2023

apotek commented Oct 25, 2023 •

edited

rfay commented Oct 25, 2023

rfay commented Oct 25, 2023

apotek commented Oct 27, 2023

rfay commented Oct 27, 2023

rfay commented Oct 30, 2023 •

edited

github-actions bot commented Oct 30, 2023

rfay commented Oct 30, 2023

rfay left a comment

apotek commented Oct 31, 2023 •

edited

rfay commented Oct 31, 2023

apotek commented Oct 31, 2023 •

edited

rfay commented Oct 31, 2023

fix: add correct mimetype for xsl and xslt, fixes #5457 #5458

fix: add correct mimetype for xsl and xslt, fixes #5457 #5458

Conversation

apotek commented Oct 25, 2023 • edited

The Issue

How This PR Solves The Issue

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

rfay commented Oct 25, 2023

apotek commented Oct 25, 2023 • edited

rfay commented Oct 25, 2023

rfay commented Oct 25, 2023

apotek commented Oct 27, 2023

rfay commented Oct 27, 2023

rfay commented Oct 30, 2023 • edited

github-actions bot commented Oct 30, 2023

rfay commented Oct 30, 2023

rfay left a comment

Choose a reason for hiding this comment

apotek commented Oct 31, 2023 • edited

rfay commented Oct 31, 2023

apotek commented Oct 31, 2023 • edited

rfay commented Oct 31, 2023

apotek commented Oct 25, 2023 •

edited

apotek commented Oct 25, 2023 •

edited

rfay commented Oct 30, 2023 •

edited

apotek commented Oct 31, 2023 •

edited

apotek commented Oct 31, 2023 •

edited