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.

Sign up for GitHub

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

Jump to bottom

IgnoreURLs doesn't work on relative URLs #168

Closed
mtlynch opened this issue May 14, 2021 · 8 comments
Closed

IgnoreURLs doesn't work on relative URLs #168

mtlynch opened this issue May 14, 2021 · 8 comments
Labels
bug

Comments

@mtlynch
Copy link
Contributor

mtlynch commented May 14, 2021

Describe the bug

I have a server-side redirect on my production site, but on the static site, it looks like a broken link. I want to tell htmltest to ignore it, but I can't figure out how. Using IgnoreURLs seems to have no effect on a relative URL

To Reproduce

  1. Create an HTML file with a non-existent relative link
  2. Add the relative URL to IgnoreURLs

.htmltest.yml

DirectoryPath: dist
IgnoreURLs:
  - /foo/bar

Source files

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width">
  <title>Demo</title>
</head>

<body>
  <a href="/foo/bar">Foo bar</a>
</body>
</html>

Expected behaviour

htmltest ignores the broken /foo/bar relative link.

Actual behaviour

htmltest reports error: target does not exist --- index.html --> /foo/bar

Even if I change the IgnoreURLs to .*/foo/bar, it still flags this as a broken link. I'm not sure if this is a bug in htmltest or if I'm misunderstanding how to specify IgnoreURLs for relative links.

Versions

  • OS: Ubuntu 20.04
  • htmltest: htmltest v0.14.0, 2021-01-23T18:38:58Z

Additional context

Thanks so much for your work on htmltest! I'd love to make a financial donation to the project if that's an option.
@mtlynch mtlynch added the bug label May 14, 2021
@divinerites
Copy link
Contributor

divinerites commented May 20, 2021 •
edited
Loading

I can confirm this bug too. thanks
htmltest 0.14.0 on MacOSX 10.14.6

@divinerites
Copy link
Contributor

divinerites commented May 23, 2021 •
edited
Loading

I'm no good at golang, but looking at the source, it seems that the .isURLIgnored function is called by checkExternal & enforceHTTPS.
Should also be called by (not sure if it is the right one) checkLink. And may be (again not sure) in checkGenericRef too.

@divinerites
Copy link
Contributor

divinerites commented May 23, 2021 •
edited
Loading

checkLink

Not that simple because isURLIgnored does a regexp on the local path, so this is too violent.

@divinerites
Copy link
Contributor

OK. I made a small patch with a new option IgnoreInternalURLs for those use case. Seems to work fine.
I will look how to proper submit a PR

@divinerites
Copy link
Contributor

Sorry but i do not succeed to submit a PR. My branch ignore-local-urls seems to be refused (permission) when I try to push it.
Can you let me know how I could do this in the best way for you ? Thanks?

git --no-optional-locks -c color.branch=false -c color.diff=false -c color.status=false -c diff.mnemonicprefix=false -c core.quotepath=false -c credential.helper=sourcetree push -v --tags --set-upstream origin refs/heads/ignore-local-urls:refs/heads/ignore-local-urls 
Pushing to https://github.com/wjdp/htmltest.git
remote: Permission to wjdp/htmltest.git denied to divinerites.
fatal: unable to access 'https://github.com/wjdp/htmltest.git/': The requested URL returned error: 403
Completed with errors, see above

@mtlynch
Copy link
Contributor Author

mtlynch commented May 23, 2021

@divinerites - You need to fork the repo.

image

Once you fork, you'll have permissions to push the branch to your personal fork. From there, you can make a pull request into the main repo.

divinerites added a commit to divinerites/htmltest that referenced this issue May 23, 2021
@divinerites
Add IgnoreInternalURLs option (Solve wjdp#168)
d3ffce7
@divinerites divinerites mentioned this issue May 23, 2021
Merged
@divinerites
Copy link
Contributor

Any news for my PR ?
I use it on a daily basic with great success.

@divinerites
Copy link
Contributor

Solved by #169

@wjdp wjdp closed this as completed Aug 17, 2021
@chalin chalin mentioned this issue Sep 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@wjdp @mtlynch @divinerites