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

Links in HTML source followed by "), ">, ') or '> returns a wrong end index #3

Closed
manuelleduc opened this issue Nov 19, 2015 · 11 comments

Comments

@manuelleduc
Copy link

I tried to extract links from https://fr.yahoo.com/?p=us for a test.

Some of the returned links contained too many characters at the end, for example :

I'm trying to understand UrlScanner's code but I'm not sure to be able to fix it.

I'll send a pull request later if I manage to fix it.

@robinst
Copy link
Owner

robinst commented Dec 1, 2015

Hey! Sorry for the late reply (I was on holidays).

It looks like you're trying to extract links from HTML source? That's not a goal of this library at the moment. It's for extracting links from plain text written by humans, such as markup formats.

If you want to extract links from the text contents of HTML, I recommend using e.g. jsoup to parse the HTML, and then running autolink-java on the text nodes.

Note that rinku (which was the inspiration for this library) tries to detect HTML tags to exclude them. I would be open to having an option to enable such a feature, in case you want to contribute it. See here how this is implemented in rinku.

@robinst robinst closed this as completed Dec 1, 2015
@robinst robinst changed the title Links followed by "), ">, ') or '> returns a wrong end index Links in HTML source followed by "), ">, ') or '> returns a wrong end index Dec 1, 2015
@frmz
Copy link

frmz commented Feb 1, 2016

Very very dirty solution but adding a replaceAll("["'].*", "") to the resulting URL fixes this problem, also, library wise, it wouldn't be too difficult to stop whenever one of those 2 chars is detected

robinst added a commit that referenced this issue Feb 5, 2016
* `;` is now treated the same as `,` and `:`
* `<` and `>` now also need to match, same as other brackets
* `/` can still be within or at the end of an URL, but if it's within a
  group of other delimiters, it behaves as a delimiter

Together, these new rules result in `">`, `"/>` and `");` to be excluded
at the end of links, while hopefully not messing with the overall
heuristics too much.
@robinst
Copy link
Owner

robinst commented Feb 5, 2016

Ok, I've taken a stab at implementing this.

@manuelleduc and @frmz, feel free to look at the test cases in PR #4 and give feedback. Basically, all the above cases now work as expected, except this one: https://s.yimg.com/os/uh-icons/0.1.16/uh/fonts/uh.eot?);src:url(https://s.yimg.com/os/uh-icons/0.1.16/uh/fonts/uh.eot?#iefix

@robinst robinst reopened this Feb 5, 2016
@frmz
Copy link

frmz commented Feb 5, 2016

That works however i don't understand why you cannot just stop when a " or ' is found, i mean, a standard, properly encoded URL shouldn't contain those 2 chars, do you have situations where URLs contains single / double quotes?

@robinst
Copy link
Owner

robinst commented Feb 7, 2016

@frmz Sure, how about this one?: https://en.wiktionary.org/wiki/it's (note how GitHub recognizes it)

Both RFC 3986 and 1738 allow '. If you can point me to a document that says otherwise, I'd love to read it. The situation for " is less clear, though it seems like they should be treated the same.

@frmz
Copy link

frmz commented Feb 8, 2016

Yeah you are right, i was actually thinking about double quotes ("), single quotes in HTML shouldn't really be used (although they are), even i found quite a lot of links with single quotes around i did not found any with a non escaped double quote inside, so i would argue that, at least ", can be safely considered a delimiter.

@robinst
Copy link
Owner

robinst commented Feb 8, 2016

Not sure. What about this?: https://en.wiktionary.org/wiki/"_"

@frmz
Copy link

frmz commented Feb 8, 2016

If you look at this very page source code you will see that inside the href the link is correctly escaped as "https://en.wiktionary.org/wiki/%22_%22", the quotes are only in the text part of the link which is not the link itself (off course a text dump of this page will still have the quotes). Real problem i see is that my browser is not escaping them in the URL bar so it might still represent an usable char.

Anyway i am still fine with implementaion above, that works in most cases, even if when scraping html probably a better solution would be using something like s/href="([^"]*)"/\1/ to get the link (but i see we might go out of topic in this case being the library a generic implementation)

@robinst
Copy link
Owner

robinst commented Feb 9, 2016

Sure, for HTML this is mostly true, although the following also works (not sure if actually valid according to spec): <a href='https://en.wiktionary.org/wiki/"_"'>test</a>.

But, as I've said in my first comment: This library is not about extracting links from HTML. Use a HTML parser for that. This library is for extracting links from plain text that a user might write, such as markup text or a GitHub comment. If it happens to also work for some forms of HTML, that is fine, but not an explicit goal.

robinst added a commit that referenced this issue Feb 9, 2016
Treat more special characters as trailing delimiters (#3)
@robinst
Copy link
Owner

robinst commented Feb 9, 2016

Merged PR #4, closing this.

@robinst robinst closed this as completed Feb 9, 2016
@robinst
Copy link
Owner

robinst commented Feb 10, 2016

These changes are now released in version 0.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants