Skip to content

fix issue #242 Malformed URL validator for attribute 'href' in tag A#245

Open
iransamaneh wants to merge 2 commits intoLullabot:mainfrom
iransamaneh:master
Open

fix issue #242 Malformed URL validator for attribute 'href' in tag A#245
iransamaneh wants to merge 2 commits intoLullabot:mainfrom
iransamaneh:master

Conversation

@iransamaneh
Copy link

add host check to detect malformed urls supporting ipv6, ipv4 and normal url

@iransamaneh
Copy link
Author

it's error is on tests/test-data/full-html/incorrect_custom_style.html url and there is not any relation between my changes and this error.

please accept and done my pull request. i changed related test files to my changes with.html and .html.out files extension

@iransamaneh
Copy link
Author

Error Details

There was 1 failure:

  1. AmpTest::testFiles with data set "tests/test-data/full-html/incorrect_custom_style.html" ('tests/test-data/full-html/inc...e.html', true)
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
<style amp-custom> on line 27 +- CSS syntax error in tag 'style amp-custom' - CSS Parser Error: preg_match(): Compilation failed: character value in \x{} or \o{} is too large at offset 32. + [code: CSS_SYNTAX category: AUTHOR_STYLESHEET_PROBLEM see: https://www.ampproject.org/docs/reference/spec.html#stylesheets] @@ @@ - CSS syntax error in tag 'style amp-custom' - invalid url protocol 'invalid:'. - [code: CSS_SYNTAX_INVALID_URL_PROTOCOL category: AUTHOR_STYLESHEET_PROBLEM see: https://www.ampproject.org/docs/reference/spec.html#stylesheets] - -<style amp-custom> on line 48 -- CSS syntax error in tag 'style amp-custom' - invalid url protocol 'invalid:'. - [code: CSS_SYNTAX_INVALID_URL_PROTOCOL category: AUTHOR_STYLESHEET_PROBLEM see: https://www.ampproject.org/docs/reference/spec.html#stylesheets] - -<style amp-custom> on line 51 -- CSS syntax error in tag 'style amp-custom' - invalid url protocol 'absolute:'. [code: CSS_SYNTAX_INVALID_URL_PROTOCOL category: AUTHOR_STYLESHEET_PROBLEM see: https://www.ampproject.org/docs/reference/spec.html#stylesheets] COMPONENT NAMES WITH JS PATH ------------------------------ No custom amp script includes required ' /home/travis/build/Lullabot/amp-library/tests/AmpTest.php:56 FAILURES! Tests: 61, Assertions: 42, Errors: 19, Failures: 1. The command "vendor/bin/phpunit tests" exited with 2. Done. Your build exited with 1.

@iransamaneh
Copy link
Author

Hi. anybody there!

@ghost
Copy link

ghost commented Jun 26, 2020

If you look at the Travis build you can see that the change broke some tests. One test that broke is an example that uses https://ad as the url in the sample, which wasn't intended to be a test for a valid url. The test would need to be updated.

Also the crazy regex looks pretty daunting. I wouldn't want to add that in without a bunch of tests that it correctly handles all kinds of urls to be sure it isn't going to create havoc on sites that work fine now.

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

Successfully merging this pull request may close these issues.

1 participant