marshmallow-code/marshmallow

fields.URL should allow relative-only validation

gh-andre opened this issue · 3 comments

gh-andre commented

Relative URLs may be used to redirect the user within the site, such as to sign in, and allowing absolute URLs without extra validation opens up a possibility of nefarious redirects.

Current fields.URL(relative = True) allows relative URLs in addition to absolute URLs, so one must set up extra validation to catch either all absolute URLs or just those that don't have a valid domain names.

It would be helpful if there was a way to set up URL validation to allow only relative URLs.

One quick and dirty way to do this would be if there was a validate.Not operator, then at the expense of matching the value twice, it would be possible to use something like this:

fields.URL(relative = True, validate=validate.Not(validate.URL()))

EDIT: Never mind the crossed out thought above - failed validations are handled only via exceptions and while failing the inner validator works in general, it requires suppressing exception handlers and is just not a good way to go about it.

lafrech commented

Maybe an additional flag absolute that would default to True?

Would you like to work on this?

gh-andre commented

@lafrech Thank you for commenting.

My thinking would be that flavors could be selected individually, as if flags are used, so they could be combined. Something along these lines:

args = {
    "ref": fields.URL(kind=fields.URL.absolute | fields.URL.relative)
    # OR
    "ref": fields.URL(kind=["absolute", "relative"])

This also would allow to retain backward compatibility for existing relative=True|False, which would be translated to these flags combined or just absolute flag being used.

An extra Boolean would work fine as well. It would be similar to how Joi handles it:

https://joi.dev/api/?v=17.9.1#stringurioptions

As for me fixing it, I'm not sure - the evaluation of the attribute for relative URLs is integrated in a pretty intricate regex construction with those Booleans used as keys. This change sounds bigger than I could commit to at this point. My apologies. Please feel free to close this issue if there's no one else interested in it.

lafrech commented

I'm not very proficient with regexes, although the change involved here should be marginal (the regex is already conditional due to the absolute flag).

I won't take the time to do it but I don't mind keeping this open for now.

  • 👍1