search

PreTeXtBook / pretext

PreTeXt: an authoring and publishing system for scholarly documents
https://pretextbook.org
Other
266 stars 209 forks source link

Warn about trailing slashes in "webwork.server" #866

Open rbeezer opened 6 years ago

rbeezer commented 6 years ago

See PR #865. Once WW refactor settles down, we could add a check and warning within the code.

Alex-Jordan commented 6 years ago

Something to consider is that when the $webwork-server is used as an argument to checkOrigin, the XSLT could remove any trailing slash. That could be done instead of, or in addition to, having the code produce a warning.

rbeezer commented 6 years ago

Even better! Maybe both: "..., so a trailing slash is being removed"

On 05/05/2018 10:20 PM, Alex Jordan wrote:

Something to consider is that when the |$webwork-server| is used as an argument to |checkOrigin|, the XSLT could remove any trailing slash. That could be done instead of, or in addition to, having the code produce a warning.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/issues/866#issuecomment-386854817, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2cgUGt1QHYSIVLAoPDWmC7eW_7sMVks5tvogngaJpZM4Tz2iC.

Alex-Jordan commented 4 years ago

I was looking into addressing this, but I think the issue went away with the big webwork refactor. The issue here was reported back when you needed to declare the server as a stringparam when building HTML. The reported issue is that using a trailing slash in the HTML would cause trouble.

But with the refactor, the only time you declare a server URL is when you do the extraction. The URL is (sort of) sanitized there, with a slash added even if one was missing, and then everything is assuming the appropriate number of slashes.

So it's not a problem now. Now it's fine to use https://webwork-ptx.aimath.org or https://webwork-ptx.aimath.org/. However it might be a problem if someone used https://webwork-ptx.aimath.org//. I could change the sanitize_url subroutine in mbx to use tools from urllib to really sanitize the input. (Actually, it's only used in one place. I'm not sure there is a real need to modularize it. So I could just get rid of it?) How does that sound?

rbeezer commented 4 years ago

You want to get rid of sanitize_url, right? I never am sure what "it" is. ;-)

If it is only used once, then yes, let's trash it. It can always rise from the dead via the git history.

Alex-Jordan commented 4 years ago

Yes, I mean that I could get rid of sanitize_url since I'm not sure it is worth the modularization. Either way, there would be a small chunk of code that does a better job of sanitizing a url. Just maybe more in an inline fashion if sanitize_url goes away.

rbeezer commented 4 years ago

Sounds good. Maybe comment that the good chunk could be modularized if it ever gets recycled.

On 12/8/19 2:08 PM, Alex Jordan wrote:

Yes, I mean that I could get rid of |sanitize_url| since I'm not sure it is worth the modularization. Either way, there would be a small chunk of code that does a better job of sanitizing a url. Just maybe more in an inline fashion if |sanitize_url| goes away.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/issues/866?email_source=notifications&email_token=AAOLM4WKJF42XE4W6A2JVBTQXVV7LA5CNFSM4E6PNCBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGHLNLY#issuecomment-563001007, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOLM4VVCIYVQX2BYH7RWD3QXVV7LANCNFSM4E6PNCBA.