Skip to content
This repository has been archived by the owner on Nov 6, 2023. It is now read-only.

[UPS.com] Reenabled Exceptions #1101

Merged
merged 5 commits into from Feb 25, 2015
Merged

[UPS.com] Reenabled Exceptions #1101

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
31c8d06
[UPS.com] Reenabled Exceptions
StevenRoddis Feb 20, 2015
c8575e8
[UPS.com] Converted negative exception to rule
StevenRoddis Feb 20, 2015
217d6b2
[UPS.com] Added additional test urls
StevenRoddis Feb 20, 2015
4174cc9
[UPS.com] Fix up the to part of the rule.
StevenRoddis Feb 25, 2015
61ea458
[UPS.com] Fixed up exclusion to apply to the correct subdomain
StevenRoddis Feb 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 27 additions & 13 deletions src/chrome/content/rules/UPS.com.xml
View file
Open in desktop
Expand Up @@ -22,28 +22,42 @@
<ruleset name="UPS.com (partial)">

<target host="ups.com" />
<target host="*.ups.com" />
<exclusion pattern="^http://ups.com/$" />
<target host="www.ups.com" />
<exclusion pattern="^http://www.ups.com/$" />
<target host="wwwapps.ups.com" />
<target host="www.apps.ups.com" />
<target host="campusship.ups.com" />
<target host="www.campusship.ups.com" />

<!--
Redirects to http:
-->
<!--exclusion pattern="^http://www\.ups\.com/content/globalentry$" /-->
<!--
all ur pkgs r now belong to nsa
<exclusion pattern="^http://www\.ups\.com/content/globalentry$" />
<test url="http://www.ups.com/content/globalentry" />

(Now redirects to login:
-->
<!--exclusion pattern="^http://www\.ups\.com/WebTracking/track($|\?)" /-->
<!--
Exceptions:
-->
<exclusion pattern="^http://www\.ups\.com/+(?!assets/|favicon\.ico|img/|styles/|stylesheets/)" />

Now redirects to login:
-->
<exclusion pattern="^http://www\.ups\.com/WebTracking/track($|\?)" />
<test url="http://www.ups.com/WebTracking/track" />
<test url="http://www.ups.com/WebTracking/track?" />
Copy link
Member

Choose a reason for hiding this comment

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

Ideally for these types of test URLs we should have query parameters that are meaningful. The goal is not just to satisfy the letter of the coverage rule, but to let the coverage rule be a reminder that the more complex the rewrite rule is, the more likely it is that one of the types of pages it covers will break, so we want to test all the different types of pages.

It may not be possible to get a useful tracking URL that does not expose private data, though. In this case I'd be willing to merge non-valid test URLs for that reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exclusions were already there just commented out, unfortunately I don't have UPS tracking number handy.

I did try 123456

Linkname: UPS: Tracking Information
URL: http://wwwapps.ups.com/WebTracking/track
Post Data:
loc=en_US&HTMLVersion=5.0&USER_HISTORY_LIST=&trackNums=123456&track.x=Track
Post Content Type: application/x-www-form-urlencoded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://wwwapps.ups.com/WebTracking/track
HTTP/1.1 302 Found
Date: Wed, 25 Feb 2015 07:46:51 GMT
Server: Apache
X-Frame-Options: SAMEORIGIN
Location: https://www.ups.com/one-to-one/login?returnto=https%3a//wwwapps.ups.com/WebTracking/track&reasonCode=-1&appid=ETRACK

<test url="http://www.ups.com/WebTracking/track?foo" />

<securecookie host="^www\.campusship\.ups\.com$" name=".+" />


<rule from="^http://(?:www\.)?ups\.com/"
<rule from="^http://(?:www\.)?ups\.com/(assets/|favicon\.ico|img/|styles/|stylesheets/)"
to="https://www.ups.com/" />
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to add a $2 to copy over the path, right? Have you manually tested this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested each of those paths in my browser.
I'll remove change (?:www\.) -> (www\.) and use $2

<test url="http://ups.com/assets/" />
<test url="http://ups.com/favicon.ico" />
<test url="http://ups.com/img/" />
<test url="http://ups.com/styles/" />
<test url="http://ups.com/stylesheets/" />
<test url="http://www.ups.com/assets/" />
<test url="http://www.ups.com/favicon.ico" />
<test url="http://www.ups.com/img/" />
<test url="http://www.ups.com/styles/" />
<test url="http://www.ups.com/stylesheets/" />

<rule from="^http://www(\.)?apps\.ups\.com/"
to="https://www$1apps.ups.com/" />
Expand Down