@weems Thanks for the pull request.

@fuglede @J0WI I'm not sure why the test is giving this error:

ERROR src/chrome/content/rules/ITunes.com.xml: Fetch error: http://itunes.com/ => https://itunes.com/: Redirect for 'https://itunes.com/' missing Location

I can reproduce it locally with fetch-test.sh, but it works fine in Firefox and the response has a Location header. I haven't investigated the code though. Ideas?

I can reproduce using curl., but I have no proper solution for it.

I think I see it. This should look for the key Location, not location. Give me a little while to look into it more.

EDIT: never mind, it's already lowercasing the key in the previous line.

@jeremyn is this a error with the test or my changes?

@weems The test.

It's still the part of the code I linked above. The regex is bad:

(Pdb) self._headerRe
regex.Regex('(?P<name>\\S+?): (?P<value>.*?)\\r\\n', flags=regex.V0)
(Pdb) headerStr
'HTTP/1.1 301 Moved Permanently\nServer: Apache\nDate: Tue Jun  1 12:48:03 PDT 1999 PDT\nReferer: http://itunes.com/\nLocation: http://www.apple.com/itunes/?cid=OAS-US-DOMAINS-itunes.com\nContent-type: text/html\nContent-length: 311\n\n'
(Pdb) headers = self._headerRe.findall(headerStr)
(Pdb) headers
[]
(Pdb)

By the way there are a lot of rules that have excluded URLs due to missing Location, see here. We really should fix it though.

@jeremyn so it's a problem with the code that checks the xml to make sure it will work?

@jeremyn ah.

Pull request #6878 should fix the test problem.

@weems The problem with the testing has been fixed by pull request #6878 so that's not a blocker anymore. Before we merge, please:

• Clean up the comments by removing lines 3-4, 9-10, and 12-13 from your modified iTunes.com.xml in 232e702

Also, there are some other itunes.com subdomains that could be included either as targets or noted as bad, for example see here. Are you willing to add them as part of this issue? I won't require it because you're just trying to fix a simple issue, but it would be nice if you could add them. Either way is fine though.

Thanks, but you deleted the wrong lines from the top. Deleting lines 3-4 should have ended up with:

<!--
    For other Apple coverage, see Apple.xml.
        Mismatch-https://www.itunes.com has a certificate mismatch
-->

Also, are you going to pass on adding the new subdomains?

Finally, we like to squash-and-merge commits, which means rewriting commits in your name. Is that all right with you for this and other pull requests you submit?

@jeremyn Apologies, I am new, I will try to get sufficient changes in. Thanks

Just a notice, closing is not a keyword which can close issues automatically after merging.

@weems I'm checking in on this pull request. Do you need anything from me to move forward?

@jeremyn I'm going to get it in this week, sorry had some distractions this week.

Hi @weems , I'm checking in again here. Do you want us to just make any remaining changes ourselves to your branch? I see you've enabled Allow edits from maintainers.

@jeremyn If you could, I'm still trying to learn the syntax here, maybe I can review for future commits. Thanks for tolerating this newbie :)

@fuglede @gloomy-ghost @J0WI I've made changes on top of @weems 's pull request. Can one of you please review this? Please do a regular merge instead of squash-and-merge because we have multiple contributors.

@gloomy-ghost I can't find any tests for https://apps.itunes.com either. I don't want to include a target, or a even a failure comment, for a domain without any URLs that return a 200.

https://apps.itunes.com/files/js/7a50ba3842fc78989e1dab4440443eb8/index.js

There we go

The URL you give in #6871 (comment) returns an HTTP 400.

https://apps.itunes.com/files/ios-music-app/ gives me an HTTP 503.

http://apps.itunes.com/files/ has some interesting information which may assist:"availableProjects: [itu-web-unsupported,atv-podcasts-app-custom,tvos-search-app,static-resources,ios-music-js-seed,itu-material-fix,ios-music-app-custom,itu-material-replace,atv-appstore-app-custom,itu-web-teacher,itu,tvos-books-app-custom,desktop-music-app-seed-custom,athena-ui,desktop-music-app-seed,desktop-music-app,itu-coursemanager,crossfire-ui,artistconnect,podcasts-connect,atv-search-app,tvos-whats-new-app,itu-universal-links,itu-web,atv-podcasts-app,atv-music-app-custom,atv-search-app-custom,appcontent,desktop-music-app-custom,atv-music-app-2.0-custom,finance-app-custom,tvos-books-app,ios-music-app-90seed,watchlist-app,ios-music-js-custom,tvos-whats-new-app-custom,ios-music-js,watchlist-app-custom,ios-music-js-seed-custom,ios-appstore-app,silverbullet-athena,atv-music-app-2.0,artistconnect-ui,atv-subscription-mgmt-app-custom,static-config,itu-sis-statics,atv-appstore-app,atv-music-app,atv-subscription-mgmt-app,tvos-search-app-custom,finance-app,atv-videos-app-custom,atv-videos-app,ios-music-app,foo]"

Retest