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

Comments

@dandv
Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2015

@mattab, see @sgiehl's comment above.

@saleemkce

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2015

  • 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

This comment has been minimized.

Copy link
Member

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 this to the 2.14.1 milestone Jul 15, 2015

@saleemkce

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2015

@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

This comment has been minimized.

Copy link
Member

commented Jul 15, 2015

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

@mattab

This comment has been minimized.

Copy link
Member

commented Jul 15, 2015

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

@mattab mattab closed this 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 mnapoli referenced this issue Jul 28, 2015
10 of 10 tasks complete
mnapoli added a commit that referenced this issue Jul 28, 2015
#7978 Improve the configuration of excluded User Agents
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

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2015

PR in #8586

mnapoli added a commit that referenced this issue Jul 29, 2015
#8448 Users can now configure excluded query parameters using line re…
…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
#8448 Users can now configure excluded IPs using line returns
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 referenced this issue Aug 3, 2015
10 of 10 tasks complete
@mnapoli mnapoli referenced this issue Aug 17, 2015
10 of 10 tasks complete

@mnapoli mnapoli removed their assignment Aug 27, 2015

@tsteur

This comment has been minimized.

Copy link
Member

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 label Oct 6, 2017

@mattab

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Edit see also #14880

@mattab mattab added Bug and removed Task labels Sep 10, 2019

@mattab mattab removed the Bug 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
Projects
None yet
7 participants
You can’t perform that action at this time.