facelessuser / pymdown-extensions

Extensions for Python Markdown
https://facelessuser.github.io/pymdown-extensions/
Other
949 stars 253 forks source link

fix: allow downloading when content-length header is missing #2413

Closed maartenbreddels closed 2 months ago

maartenbreddels commented 2 months ago

This can happen for valid http responses, such as when the server is using chunked encoding.

Fixes #2404.

maartenbreddels commented 2 months ago

This makes the following work

--8<-- "https://py.cafe/files/maartenbreddels/altair-car-performance-comparison/app.py"

Which gives these headers:

$ curl -I https://py.cafe/files/maartenbreddels/altair-car-performance-comparison/app.py                                                   fix_download_without_content_length_header ◼
HTTP/2 200 
age: 0
cache-control: public, max-age=0, must-revalidate
content-type: text/plain
date: Sun, 14 Jul 2024 19:45:46 GMT
server: Vercel
strict-transport-security: max-age=63072000
vary: RSC, Next-Router-State-Tree, Next-Router-Prefetch
x-matched-path: /files/[username]/[projectname]/[...path]
x-vercel-cache: MISS
x-vercel-execution-region: iad1
x-vercel-id: fra1::iad1::zhtpq-1720986346756-476b90c31ef2

PS: this is used to render the sourcecode for the hosted app at https://py.cafe/maartenbreddels/altair-car-performance-comparison

facelessuser commented 2 months ago

Seems like a pretty reasonable approach.

We can ignore any lint errors outside of your changes (looks like some new checks are now throwing errors in old code that used to pass).

Tests will have to be updated to pass for new expectations as before we used to always error out for missing content length. I imagine we will likely need at least one new test to cover this new use case as well. If this seems like a little beyond what you are comfortable doing (as it will probably involve doing some mocking and such), I can probably take what you have and finish, but feel free to update if you are comfortable in this area. Either way, I think this is a decent way to move forward on this.

maartenbreddels commented 2 months ago

Hi,

no problem with adding the test. I managed to get the unit tests working. Let me know if you prefer to take over and change it to your liking, if so, feel free to force push on my branch.

Hope this is to your liking.

Regards,

Maarten

facelessuser commented 2 months ago

Overall it looks good, but I'd made a few comments.

maartenbreddels commented 2 months ago

Good comments 👍 . I implemented them as two separate commits to help review. Feel free to squash merge it when ready.

facelessuser commented 2 months ago

@maartenbreddels Thanks for your work on this!

@gir-bot lgtm

maartenbreddels commented 2 months ago

Not to put pressure, but what is your release cadence?

I want to advertise to people to use pycafe + mkdocs and this plugin to have live code examples in their docs, which depends on this fix.

Could you let me know here or in #2404 when this is released, then I can let people know this is possible.

Thanks!

facelessuser commented 2 months ago

I imagine it will be sometime soon. I can't promise I'll remember to comment. The best way to know when a release is made is to watch the repo, and if all you care about is releases, configure it to only watch releases. You can disable the watch once the release in question is made.

maartenbreddels commented 2 months ago

Really good idea, will do that! Thanks a ton, also, for these great extensions, and from what I've seen a very nice and high-quality codebase. Cheers