Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excluded User Agent still tracked #7978

Open
dandv opened this issue May 23, 2015 · 13 comments
Open

Excluded User Agent still tracked #7978

dandv opened this issue May 23, 2015 · 13 comments
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.

Comments

@dandv
Copy link
Contributor

dandv commented May 23, 2015

I had configured the following string under the Websites -> Global list of user agents to exclude:

"Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0"

image

It belongs to the Monitor.us service.

Yet later I still saw that bot in the Real-time Visitors:

image

Do I maybe need to add the user agent string without quotes? By the way, there's no explanation of the format in which UA strings should be entered in that box. Presumably, one per line, but who knows? CSV format? Quoted or not?

@sgiehl
Copy link
Member

sgiehl commented May 24, 2015

Looking at the code it seems to be a unquoted comma separated list. But that might not be that good at all, as some useragents might also contain a comma.
Nevertheless it should be explained which format is used.

@mattab
Copy link
Member

mattab commented May 24, 2015

Do I maybe need to add the user agent string without quotes?

Yes you need to set the user agent without quote. Does it work for you after removing the double quotes?

@dandv
Copy link
Contributor Author

dandv commented May 24, 2015

IMO it would be most intuitive and easiest to paste user agents one per line, and have Piwik automatically take care of enclosing double quotes. These are the two ways users will find the UA strings, from various sites (without quotes), or from server logs (with quotes).

Comma-separate adds unnecessary complexity and potential for breakage if comas aren't escaped correctly.

@mattab
Copy link
Member

mattab commented May 24, 2015

Actually the textarea field should accept one user agent per line, not comma separated. If one user agent per line does not work, please let us know, it's a bug we would fix ASAP

@dandv
Copy link
Contributor Author

dandv commented May 24, 2015

@mattab, see @sgiehl's comment above.

@saleemkce
Copy link
Contributor

  • current good commit b8cafee .Previous commit references/branches that you see here were deleted because they included some worng merge-commits in my local master.

IMO it would be most intuitive and easiest to paste user agents one per line, and have Piwik automatically take care of enclosing double quotes.

  • This has been fixed. Fix will take care of quotes if present

Actually the textarea field should accept one user agent per line, not comma separated. If one user agent per line does not work, please let us know, it's a bug we would fix ASAP

  • I tested this scenario extensively with 4 different browsers (IE, Chrome, FF & Opera)/Windows OS today. I didn't face any incorrect tracking. But when quotes were present before the commit I made, it failed to work correctly.
  • Tested browsers in my system
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20121202 Firefox/17.0 Iceweasel/17.0.1 (Ubuntu browser just for testing)
Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML like Gecko) Chrome/43.0.2357.81 Safari/537.36 (Chrome)
Mozilla/5.0 (Windows NT 6.1; rv:35.0) Gecko/20100101 Firefox/35.0 (Mozilla)
Opera/9.80 (Windows NT 6.1; U; en) Presto/2.10.229 Version/11.61 (Opera)
Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; WOW64; Trident/6.0)  (IE)

@mattab
Copy link
Member

mattab commented Jul 15, 2015

Suggested fix:

  • when user inputs a user agent with double quotes, we remove the enclosing double quotes (+ trim) before testing against the visitor user agent

In other words: when user forgets to remove enclosing double quotes, we do it for her

@mattab mattab added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. labels Jul 15, 2015
@mattab mattab added this to the 2.14.1 milestone Jul 15, 2015
@saleemkce
Copy link
Contributor

@mattab The fix I posted handles the "remove the enclosing double quotes" part. And the function checkAndReturnCommaSeparatedStringList already does the "(+ trim)" part. So, could you suggest what else we should consider or is the current fix sufficient?

@sgiehl
Copy link
Member

sgiehl commented Jul 15, 2015

as mentioned in #8050 useragents might contain quotes, so simply removing them is not accurate

@mattab
Copy link
Member

mattab commented Jul 15, 2015

we should remove the enclosing quotes only, eg. using the second parameter of PHP trim function

@mattab mattab closed this as completed Jul 15, 2015
@mattab mattab reopened this Jul 15, 2015
@barbushin barbushin self-assigned this Jul 16, 2015
@mattab mattab modified the milestones: 2.14.1, Short term Jul 16, 2015
@mattab mattab modified the milestones: 2.15.0, 2.14.1 Jul 16, 2015
@barbushin barbushin removed their assignment Jul 23, 2015
@mnapoli mnapoli self-assigned this Jul 27, 2015
mnapoli added a commit that referenced this issue Jul 28, 2015
Before this commit, users can configure excluded user agents by separating them with commas. Enclosing quotes are not trimmed.

With this commit, users can configure excluded user agents one per line. Enclosing quotes are trimmed. This is meant to be simpler and more consistent. It also allows to exclude user agents containing both commas and quotes. This change applies both to global settings as well as site-specific settings.

This commit also changes the help text, which now explicitly says that user agents should be separated by new lines.

A migration script is included: it will replace commas by new lines.

Related: #7978, #8050
@mnapoli
Copy link
Contributor

mnapoli commented Jul 28, 2015

PR in #8586

mnapoli added a commit that referenced this issue Jul 29, 2015
…turns

Before this commit, users can configure excluded query parameters by separating them with commas.

With this commit, users can configure them one per line. This is meant to be simpler and more consistent (see #8448). This is now also consistent with form inline helps. This change applies both to global settings as well as site-specific settings.

A migration script is included: it will replace commas by new lines.

Related: #7978, #8050
mnapoli added a commit that referenced this issue Jul 29, 2015
Before this commit, users can configure excluded IPs by separating them with commas.

With this commit, users can configure them one per line. This is meant to be simpler and more consistent (see #8448). This is now also consistent with form inline helps. This change applies both to global settings as well as site-specific settings.

A migration script is included: it will replace commas by new lines.

Related: #7978, #8050
@mnapoli mnapoli removed their assignment Aug 27, 2015
@tsteur
Copy link
Member

tsteur commented Aug 29, 2015

There is a PR for that but it's targeting 3.0 . Can we maybe move this issue out of 2.15 into 3.0? Maybe a solution that is ok for now would be to just mention in the inline help to not put them into quotes?

@mattab mattab modified the milestones: 3.0.0, 2.15.0 Aug 30, 2015
@mattab mattab added the Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement. label Oct 6, 2017
@mattab
Copy link
Member

mattab commented Sep 10, 2019

Edit see also #14880

@mattab mattab added Bug For errors / faults / flaws / inconsistencies etc. and removed Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. labels Sep 10, 2019
@mattab mattab removed the Bug For errors / faults / flaws / inconsistencies etc. label Sep 10, 2019
@mattab mattab modified the milestones: 3.12.0, Backlog (Help wanted) Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.
Projects
None yet
Development

No branches or pull requests

7 participants