facelessuser / pymdown-extensions

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

Download snippet + server reply without content-length #2404

Closed maartenbreddels closed 2 days ago

maartenbreddels commented 1 week ago

Description

Hi,

When I use:

        --8<-- "http://localhost:3001/files/maartenbreddels/test/app.py"

But my server replies with:

$ curl -I http://localhost:3001/files/maartenbreddels/test/app.py
HTTP/1.1 200 OK
cache-control: public, max-age=0, must-revalidate
server: Vercel
x-vercel-id: dev1::qcjlo-1720123058986-b8c5fdc87ba8
x-vercel-cache: MISS
vary: RSC, Next-Router-State-Tree, Next-Router-Prefetch, Accept-Encoding
content-type: text/plain
date: Thu, 04 Jul 2024 19:57:39 GMT
keep-alive: timeout=5
connection: close

(i.e. no content-length - which is also possible with transfer-encoding: chunked)

This extension raises the error:

                raise ValueError("Missing content-length header")

A possible solution could be not to do the check when url_max_size==0.

Let me know what you think.

Regards,

Maarten

Minimal Reproduction

I cannot find a public server that does this, so difficult to give you reproducible steps now

Version(s) & System Info

markdown-it-py 3.0.0

facelessuser commented 1 week ago

@maartenbreddels does ignoring the max size allow this to pass?

maartenbreddels commented 1 week ago

If you mean ignoring that check, yes. I commented out the code and then this feature works for me. I’m curious why the max size check is there actually?

facelessuser commented 1 week ago

To prevent a link to a massive file that could take a very long time to download. We assume people may enable this feature on live servers (which we don't recommend) and it may be abused in all sorts of ways, so we give sane defaults by...well default.

While we disable the check when max is set to zero, I did not anticipate that the the length may be absent in some cases.

People are free to remove max requirement. If you remove the requirement, you do so at your own risk. In very controlled environments, this is likely less of a concern.

maartenbreddels commented 1 week ago

Ok, thank you for the explanation.

note that with ‘url_max_size==0‘ the code still raises an error. But skipping the check in that case might be a reasonable fix for my use case.

facelessuser commented 1 week ago

What error? These are the kind of things I'm looking for.

We already ignore the check if max_url_size is zero: https://github.com/facelessuser/pymdown-extensions/blob/main/pymdownx/snippets.py#L207.

The error you posted is because we check if the return data from the request has content-length, something I did not anticipate would be missing in some cases.

If a max size is requested, we intend to fail if that is missing. So, moving forward, I imagine if content-length is missing, and we have a limit of zero, we can just process the lines in the request and exit. I imagine this will clear up errors assuming there is not something else that is fundamentally broken with the request. If that is the case, then we need to circle back and understand the real issue.

facelessuser commented 1 week ago

Considering that I cannot easily reproduce this case, I would need someone like you, who can reproduce this issue, to propose a validated fix.