Opened 3 years ago

Closed 3 months ago

#4851 closed defect (rejected)

Redirect filter list comment not ignored if same address

Reported by: rhana@… Assignee:
Priority: Unknown Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: greiner, sergz Blocked By:
Blocking: Platform: Chrome
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

One of our users reports:

Syntax "! Redirect" of AdBlock Plus filter is not correctly implemented.

https://adblockplus.org/en/filters

"This comment is ignored if the new address is the same as the current address, meaning that it can be used to enforce the "canonical" address of the filter list."

What actually happened in AdBlock is it seems to stuck in the loop for a while, quickly realized it, but returned an error instead of just ignore this line and continue reading the rest.

He originally reported this in AdBlock 3.6.0 for Chrome beta. He reported on January 22 that it's still happening, in both the beta and stable releases.

According to our dev (Brent Montrose):

I find it difficult to believe that the ABP Core code wouldn't handling redirects correctly, so it maybe the user's misinterpreting the documentation, etc. But you should probably open a ABP ticket / issue.

I'll get more information about the error he's seeing. In the meantime, does this sound like a bug or a misreading of the filter guide?

Change History (8)

comment:1 Changed 3 years ago by rhana@…

Note to self: This is https://help.getadblock.com/helpdesk/tickets/8791 for reference

comment:2 Changed 3 years ago by greiner

  • Cc trev greiner added
  • Component changed from Unknown to Core
  • Summary changed from "!Redirect" to Redirect filter list comment not ignored if same address

After having a quick look at the code I couldn't find where the redirect is supposed to be ignored if it's the same address. I'll keep looking and I'll also investigate whether or not this is a regression.

In the meantime Wladimir might have some further background info on the intended behavior of that filter list comment so I added him to CC.

comment:3 Changed 3 years ago by greiner

It looks like it's a regression that's caused by the refactoring of the synchronizer logic in 2013. It removed an if-statement which appears to have been responsible for enforcing what's mentioned in the documentation as well as another if-statement which does a similar check.

Note that based on our automated tests, a redirection loop is supposed to result in an error so we should probably update the online documentation to reflect that rather than reintroducing unnecessary behavior.

Last edited 3 years ago by greiner (previous) (diff)

comment:4 Changed 3 years ago by rhana@…

By the way, dunno if it helps, but the error the user is getting is "Failed, download failure."

His custom filter file is here: https://raw.githubusercontent.com/littlepig10/92Clearnet/master/AdBlock/AdBlock-debug-92clearnet.txt

comment:5 Changed 3 years ago by rhana@…

http://imgur.com/a/it40G

comment:6 Changed 2 years ago by fhd

  • Cc trev removed

comment:7 Changed 23 months ago by sergz

  • Cc sergz added

comment:8 Changed 3 months ago by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from new to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Note: See TracTickets for help on using tickets.