-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
URL Validator doesn't accept Google Font URLs #23944
Comments
You are right in that the pipe causes the fail, yet it's not a bug. RFC 3986 section 3.3 specifies that a Looking further down in appendix A we find:
As the pipe is not in this list, the URL validator is right in rejecting the URL. This is actually Google's fault for not using an acceptable subdelimiter, like the commonly used ampersand in this case. This makes this issue not a bug. On your end however your users are expecting a similar behavior to what their browser does, but browsers accept far more invalid characters quietly - type a space in a URL and it will happily convert it to So if you insist on validating the URL against the RFC, it's your responsibility as the app developer to process your user's input before sending it down the validator's chain. It would be pretty safe to use Status: works for me |
Thanks @curry684, thats a very comprehensive and sensible response. I'll handle it our side and figure out how I can file a bug with Google.... |
@edhgoose I suppose we can close it then. |
Yes, happy to. |
Just noticed a small mistake in my response: the culprit is in the query component in section 3.4, not the path. However as that is validated against |
We allow users to specify a URL to link to fonts. We mandate that that URL is an https URL, and so we have the following assertion:
If you specify a URL like: "https://fonts.googleapis.com/css?family=Roboto:400,700|Roboto+Condensed:400,700" it'll fail.
The pipe seems to be the cause. If it contains
something=x|y
then it'll fail.URL Encoding it to
%7C
seems to work for google fonts, and so maybe the issue is that we need to url encode the|
symbol, but from a non-technical users point of view that is confusing.The text was updated successfully, but these errors were encountered: