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

merged 5 commits into from Feb 25, 2015

Conversation

fuzzyroddis
Copy link
Contributor

Fixes #1039


<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

-->
<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

@jsha
Copy link
Member

jsha commented Feb 25, 2015

Looks like tests are failing because you are missing ".ups.com" after www(.)?apps.

@fuzzyroddis
Copy link
Contributor Author

Fixed, I probably should stop coding while tired.

jsha added a commit that referenced this pull request Feb 25, 2015
[UPS.com] Reenabled Exceptions
@jsha jsha merged commit d420bbf into EFForg:master Feb 25, 2015
@fuzzyroddis fuzzyroddis deleted the ups branch March 8, 2015 08:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track a package using UPS - requires login
3 participants