Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#1574 closed defect (fixed)

Apparently we break Request Policy

Reported by: pde Owned by: pde
Priority: High Milestone:
Component: HTTPS Everywhere/EFF-HTTPS Everywhere Version:
Severity: Keywords:
Cc: jsamuel, mikeperry, keb Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

From Justin Samuel:

Thanks for making HTTPS Everywhere. It would be great if it could be
compatible with extensions that needs to know when URIs get rewritten.

For example, my extension (RequestPolicy) currently has no way of
knowing that rewritten requests from link clicks should be allowed. As a result, things like clicking on wikipedia links in google results fail, which is a bummer. (As a warning in case you decide to test this particular example: they do work on the second click because HTTPS Everywhere changes the request context's src when rewriting).

Attached is a patch for HTTPS Everywhere that sends observer service
notifications for URI rewrites. Let me know if there's anything I an
do to help get this patch or an equivalent added. Note that the most
reliable and simplest place I could see to add the notification was in RuleSet.prototype.replaceURI(). If you have better suggestions, let me know. Extra notifications are preferable to missed or after-the-fact notifications.

Child Tickets

Attachments (3)

notify_observers.diff (1.5 KB) - added by pde 9 years ago.
notify_observers.unified.diff (2.4 KB) - added by jsamuel 9 years ago.
same diff, but in unified format
notify_observers.2.diff (2.5 KB) - added by jsamuel 9 years ago.
Refreshed patch.

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by pde

Attachment: notify_observers.diff added

Changed 9 years ago by jsamuel

same diff, but in unified format

comment:1 Changed 9 years ago by mikeperry

Cc: jsamuel added

Justin - I still don't see you listening for this event yet in request policy 0.5.15b2. Is this the solution you want?

Changed 9 years ago by jsamuel

Attachment: notify_observers.2.diff added

Refreshed patch.

comment:2 Changed 9 years ago by jsamuel

RequestPolicy 0.5.16a1 now listens for the topic notified by the attached patch.

Issue tracked for RP here:
https://www.requestpolicy.com/dev/ticket/115

Latest RP with the changes:
https://www.requestpolicy.com/releases/requestpolicy-0.5.16a1.xpi

Using the beta of HTTPS Everywhere with notify_observers.2.diff applied and RequestPolicy 0.5.16a1 solves the conflict problem for me on Firefox 3.6.10. I didn't see the extension conflict with the Firefox 4 betas, so if someone has seen that then it would be good to see if it's fixed with these changes. HTTPS Everywhere's beta seemed to have problems for me on Fx 3.0.19, so I didn't test there---if that sounds unexpected, it's either something I did when testing it or something I broke with this patch (though it didn't seem likely).

comment:3 Changed 9 years ago by pde

Resolution: fixed
Status: newclosed

Justin, since Seth applied the notify_observers patch in 0.2.3.development.2 and onwards, I'm going to close this bug. If there are further things that need going, feel free to reopen.

comment:4 Changed 6 years ago by keb

Since 2.3.25-8 TBB crashes (the torbrowser process disappears) if RequestPolicy is enabled, before even going to any websites.

comment:5 Changed 6 years ago by pde

Cc: mikeperry keb added

keb, have you tested to confirm that HTTPS Everywhere + RequestPolicy is the specific fatal combination?

mikeperry, what changes landed in 2.3.25-8? (for the record I can't find a changelog or even git tags for TBB versions at https://gitweb.torproject.org/builders/tor-browser-bundle.git/shortlog)

comment:6 in reply to:  5 Changed 6 years ago by keb

Replying to pde:

keb, have you tested to confirm that HTTPS Everywhere + RequestPolicy is the specific fatal combination?

dont know about HTTPS-Everywhere.
i install RequestPolicy and TBB starts crashing, i remove it and things are okay again. it could be since the previous release -5 or -6 (i got some crashes there too but wasnt tracking add-ons carefully). i didnt put any priority on this nor reopen because TBB doesnt support the use of RequestPolicy and from discussion above it seems deprecated.

comment:7 Changed 6 years ago by keb

rereading the bug, it seems i reported in the wrong place. :(

Note: See TracTickets for help on using tickets.