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

Correctly parse all URL schemes in UrlHelper::isLookLikeUrl(). #8744

Merged
merged 3 commits into from Sep 11, 2015

Conversation

diosmosis
Copy link
Member

As title. Changed the regex to match all allowed schemes (as per the RFC: https://tools.ietf.org/html/rfc3986#section-3.1).

Fixes #8722

@diosmosis diosmosis added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Sep 9, 2015
@diosmosis diosmosis added this to the 2.15.0 milestone Sep 9, 2015
@sgiehl
Copy link
Member

sgiehl commented Sep 9, 2015

There are two integration tests failing now. Maybe we should check where the isLookLikeUrl method is used. At some points maybe it would be more correct to really check for an URL

@diosmosis
Copy link
Member Author

Fixed the failing tests. Will fix other failures if they show up.

At some points maybe it would be more correct to really check for an URL

We should probably replace this w/ a utility library in 3.0. For LTS, big changes like this would be counterproductive.

@diosmosis
Copy link
Member Author

Failing UI test is a random failure

page.evaluate(function () {
// cannot use sendKeys since quickform does not use placeholder attribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diosmosis curious why it's still above page.sendKeys('input[name=siteName]', 'Serenity'); if we cannot use sendKeys?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to delete it :)

@mattab
Copy link
Member

mattab commented Sep 11, 2015

LGTM and works!

mattab pushed a commit that referenced this pull request Sep 11, 2015
Correctly parse all URL schemes in UrlHelper::isLookLikeUrl().
@mattab mattab merged commit 301dd81 into master Sep 11, 2015
@diosmosis diosmosis deleted the 8722_correct_scheme_validation branch September 11, 2015 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants