Allow image src without scheme and with external domain - #144
Allow image src without scheme and with external domain#144fabienheureux wants to merge 6 commits into
Conversation
fabienheureux
changed the title
fabienheureux
changed the title
|
Sorry for the noise, I pushed without running tests locally, again 🙄, they now pass. @nickmoreton do you think you could take a look ? I am not very proud of this approach, especially the hardcoded file extensions, but I did not see how could we better handle these various cases. |
fabienheureux
changed the title
915fada to
240514a
Compare
|
@fabienheureux Thanks we really appreciate the pull request. @nimasmi I tested it out and is working OK. Would you take a look please and confirm happy to merge. Thanks |
nimasmi
left a comment
nimasmi
left a comment
There was a problem hiding this comment.
Thanks @fabienheureux. I'm always in favour of urlparse over string manipulation. It looks like we should remove some further custom string parsing.
| first_url_part = url.path.split("/")[0] | ||
| if ( | ||
| "." in first_url_part | ||
| and first_url_part.split(".")[1].lower() not in known_file_extensions |
There was a problem hiding this comment.
Issue: In the following case, the unrecognised local file extension is interpreted as a domain:
>>> get_absolute_src("image.bmp", "https://www.example.com")
'https://image.bmp'urlparse results have a .hostname attribute that you can use in this situation. You will have to remove the .lstrip("/") line though.
>>> from urllib.parse import urlparse
>>> urlparse("image.bmp").hostname
>>> urlparse("example.com/image.bmp").hostname
>>> urlparse("//example.com/image.bmp").hostname
'example.com'| get_absolute_src("//example.com/fakeimage.jpg", "http://www.example.com"), | ||
| "http://example.com/fakeimage.jpg", | ||
| ) | ||
|
|
There was a problem hiding this comment.
Suggestion: Add a test case for an unrecognised extension on the local domain: get_absolute_src("/fakeimage.badextension", "http://www.example.com")
|
@nickmoreton I am cleaning up old PRs. Can I close this one ? I hope I could have take @nimasmi comment into account but never found the time and I won't use this plugin in a near future. |
I'm hoping to get around to considering this soon. |
3 participants
Description
If images have a src attribute like
//example.com/image.jpg, the imported url does not care about the domain and prepend the wagtail domain, hence leading to a 404 when downloading the image