Skip to content
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

validation system false positive on URL with # #3219

Closed
jevonearth opened this issue Aug 5, 2020 · 5 comments · Fixed by #3227
Closed

validation system false positive on URL with # #3219

jevonearth opened this issue Aug 5, 2020 · 5 comments · Fixed by #3227
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. mlh Major League Hacking Fellowship status: needs more information There is not enough information to take action on the issue.

Comments

@jevonearth
Copy link

🐛 Bug Report

Validation has false positive on URLs with # in the URL

Have you read the [Contributing Guidelines on issues]

yes

To Reproduce

  1. Create a link definition in docusaurus.config.js as follows:
   footer: {
      links: [ 
        {
          title: 'Docs', 
          items: [
            {
              href: 'https://riot.im/app/#/room/#ligo-public:matrix.org',
              label: 'Riot'
            }
	  ]
	}
    }
  1. run npm run build

Expected behaviour

Build should complete without error

Actual Behavior

We get an error:

Creating an optimized production build...

A validation error occured.
The validation system was added recently to Docusaurus as an attempt to avoid user configuration errors.
We may have made some mistakes.
If you think your configuration is valid and should keep working, please open a bug report.

ValidationError: "footer.links[1].items[3].href" must be a valid uri

Your Environment

"@docusaurus/core": "^2.0.0-alpha.61"

@jevonearth jevonearth added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Aug 5, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 5, 2020

Hi,

We use Joi for this validation, with the following rule: Joi.string().uri()

If Joi refuses your URI, this is because it's not a valid URI, or Joi has a bug (in which case you should report it)

Can you give me a link from a web authority that claims an uri with 2 hashes is a valid uri? I'm not sure it is.

If it's a Joi uri validation bug, you should open an issue to the Joi validation library.
Until this is fixed, how can we solve your uri problem without relaxing the validation too much, any idea?

@slorber slorber added difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. mlh Major League Hacking Fellowship status: needs more information There is not enough information to take action on the issue. and removed status: needs triage This issue has not been triaged by maintainers labels Aug 5, 2020
@jevonearth
Copy link
Author

It's unclear to me if using # is part of the RFC spec. My cursory impression is Joi appears to be dogmatic on that.

The link in question is to a SPA. The # portions to not get transmitted to a server. The end result here is that the new docusarus validation breaks links that we have had in place for a long time.

Encoding the # with %23 will not work as a workaround, sadly.

@slorber
Copy link
Collaborator

slorber commented Aug 5, 2020

The end result here is that the new docusarus validation breaks links that we have had in place for a long time.

Sorry for that. Also worth considering that before that we didn't do any validation, so you could pass any invalid URL as well. We don't really want to revert these validation logic.


I've opened an issue on Joi because I think if new URL() can parse your URL then Joi should rather accept it (I guess?).

hapijs/joi#2435


@anshulrgoyal @teikjun as you are familiar with the validation system, what about adding something like Joi.alternatives(Joi.string().uri(),parseableUrlSchema()) or something, with parseableUrlSchema not rejecting if new URL() is ok?

@anshulrgoyal
Copy link
Contributor

we can add a Joi.custom() and check if url.parse work on the given url or not.

@slorber
Copy link
Collaborator

slorber commented Aug 5, 2020

That would be great thanks 👍

As we could use this in other places, please add it to the new package utils-validation

(also worth some tests might be refactored using methods of this package)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. mlh Major League Hacking Fellowship status: needs more information There is not enough information to take action on the issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants