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

[BUGFIX] Allow "Content-Type" in content #959

Merged
merged 2 commits into from
Mar 22, 2021
Merged

Conversation

JakeQZ
Copy link
Contributor

@JakeQZ JakeQZ commented Mar 21, 2021

Use a regular expression to determine whether the provided HTML contains a
Content-Type meta element instead of just testing for the string
"Content-Type". Also allow for case insensitivity.

A valid Content-Type meta element which is invalidly placed (i.e. within the
body) will still be determined to be valid. This will be addressed separately
for #923.

Add the package rawr/cross-data-providers to the development dependencies.
This is useful to generate a matrix of combinations for test data providers.
There are a few existing instances where this could be used to replace code
effectively doing that, but which are not addressed in this commit.

Rename affected data provider methods from xyzDataProvider to provideXyx,
which is more succinct and a more commonly used nomenclature on other projects.

Fixes #957.

Use a regular expression to determine whether the provided HTML contains a
`Content-Type` `meta` element instead of just testing for the string
"Content-Type".  Also allow for case insensitivity.

A valid `Content-Type` `meta` element which is invalidly placed (i.e. within the
`body`) will still be determined to be valid.  This will be addressed separately
for #923.

Add the package `rawr/cross-data-providers` to the development dependencies.
This is useful to generate a matrix of combinations for test data providers.
There are a few existing instances where this could be used to replace code
effectively doing that, but which are not addressed in this commit.

Rename affected data provider methods from `xyzDataProvider` to `provideXyx`,
which is more succinct and a more commonly used nomenclature on other projects.

Fixes #957.
@JakeQZ JakeQZ added the bug label Mar 21, 2021
@JakeQZ JakeQZ added this to the 5.0.1 milestone Mar 21, 2021
@JakeQZ JakeQZ self-assigned this Mar 21, 2021
- More explicity test return value from `preg_match`;
- Add tests that the regular expression does not match unexpected keywords of
  which the expected keywords are substrings.
@JakeQZ
Copy link
Contributor Author

JakeQZ commented Mar 22, 2021

I've changed the test for the preg_match return value as suggested, and also added tests to confirm the regex does not match unexpected longer keywords which contain the expected keyword as a substring.

Copy link
Contributor

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

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

LGTM.

@oliverklee oliverklee merged commit a9d7a71 into main Mar 22, 2021
@oliverklee oliverklee deleted the bugfix/content-type branch March 22, 2021 20:08
JakeQZ added a commit that referenced this pull request Apr 11, 2021
Since the library `rawr/cross-data-providers` has been added as a development
dependency (in #959), make use of it where applicable to combine datasets for
tests.  This makes the test data providers clearer and eliminates duplication
of functionality.
JakeQZ added a commit that referenced this pull request Apr 11, 2021
Since the library `rawr/cross-data-providers` has been added as a development
dependency (in #959), make use of it where applicable to combine datasets for
tests.  This makes the test data providers clearer and eliminates duplication
of functionality.
JakeQZ added a commit that referenced this pull request Apr 11, 2021
Since the library `rawr/cross-data-providers` has been added as a development
dependency (in #959), make use of it where applicable to combine datasets for
tests.  This makes the test data providers clearer and eliminates duplication
of functionality.
oliverklee pushed a commit that referenced this pull request Apr 11, 2021
Since the library `rawr/cross-data-providers` has been added as a development
dependency (in #959), make use of it where applicable to combine datasets for
tests.  This makes the test data providers clearer and eliminates duplication
of functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positives for content-type meta element presence check
2 participants