Handling of protocol relative external urls in zimcheck #307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #307 +/- ##
==========================================
+ Coverage 51.13% 51.18% +0.05%
==========================================
Files 21 21
Lines 1942 1944 +2
Branches 1151 1152 +1
==========================================
+ Hits 993 995 +2
Misses 947 947
Partials 2 2
Continue to review full report at Codecov.
|
|
@kelson42 I am glad that this PR made you laugh, but I wonder what you find so funny about it? 🤔 |
|
@veloman-yunkan My bad. Wrong emoticon. Was just happy to see a fix finally released. |
mgautierfr
requested changes
Jul 5, 2022
zimcheck treats protocol-relative absolute URLs as relative URLs. This
bug is demonstrated by enhanced test data (which makes the zimcheck test
fail). Bugfix coming in the next commit.
The test data was enhanced in the following ways:
- Added external links in main.html. The protocol-relative variant
(//openzim.org) is the one that breaks the tests. The mismatch
in the test output demonstrates that the absolute link //openzim.org
is treated as an internal link:
```
[ RUN ] zimcheck.internal_url_check_poorzimfile
.../test/zimcheck-test.cpp:249: Failure
Expected equality of these values:
expected_stdout
Which is: ...
std::string(zimcheck_output)
Which is: ...
With diff:
@@ +7,23 @@
(A/non_existent.html) were not found in article dangling_link.html
Found 1 empty links in article: empty_link.html
+ The following links:
+- //openzim.org
+(openzim.org) were not found in article empty_link.html
+ The following links:
+- //openzim.org
+(openzim.org) were not found in article external_image_http.html
+ The following links:
+- //openzim.org
+(openzim.org) were not found in article external_image_https.html
+ The following links:
+- //a.io/pic.png
+(a.io/pic.png) were not found in article external_image_protocol_relative.html
+ The following links:
+- //openzim.org
+(openzim.org) were not found in article main.html
../../oops.html is out of bounds. Article: outofbounds_link.html
+ The following links:
+- //openzim.org
+(openzim.org) were not found in article outofbounds_link.html
[INFO] Overall Test Status: Fail
[INFO] Total time taken by zimcheck: <3 seconds.\n
Test context:
zimcheck -u data/zimfiles/poor.zim
[ FAILED ] zimcheck.internal_url_check_poorzimfile
```
- Added an image with inline data. This change only increases the
functional coverage of zimcheck testing.
- In poor.zim, added three (http, https and protocol-relative) variants
of absolute referenes to an external image. The mismatch in the test
output demonstrates that the protocol-relative variant is not reported:
```
[ RUN ] zimcheck.external_url_check_poorzimfile
.../test/zimcheck-test.cpp:249: Failure
Expected equality of these values:
expected_stdout
Which is: ...
std::string(zimcheck_output)
Which is: ...
With diff:
@@ -5,5 @@
http://a.io/pic.png is an external dependence in article external_image_http.html
https://a.io/pic.png is an external dependence in article external_image_https.html
- //a.io/pic.png is an external dependence in article external_image_protocol_relative.html
[INFO] Overall Test Status: Fail
[INFO] Total time taken by zimcheck: <3 seconds.\n
Test context:
zimcheck -x data/zimfiles/poor.zim
[ FAILED ] zimcheck.external_url_check_poorzimfile (1 ms)
```
Also:
- In create_test_zimfiles script, replaced the obsolete option -f
of zimwriterfs with a new option -I.
This change fixes the bug demonstrated by the previous commit.
veloman-yunkan
force-pushed
the
zimcheck_protocol_relative_external_urls
branch
from
ee78b4b
to
3c5c32e
Compare
mgautierfr
approved these changes
Jul 5, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should fix #305