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

Scheduled report name is double encoded #7987

Closed
mattab opened this issue May 24, 2015 · 6 comments · Fixed by #11642
Closed

Scheduled report name is double encoded #7987

mattab opened this issue May 24, 2015 · 6 comments · Fixed by #11642
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@mattab
Copy link
Member

mattab commented May 24, 2015

  • Got: scheduled report name is double encoded within the scheduled report "at a glance"

report name double encoded

  • Expected: scheduled report name should not be double encoded in both PDF and HTML scheduled reports.
@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label May 24, 2015
@mattab mattab added this to the 2.14.0 milestone May 24, 2015
@diosmosis diosmosis self-assigned this May 25, 2015
@diosmosis
Copy link
Member

Issue is that API/Request.php sanitizes the $description parameter before it is stored in the DB (via UrlHelper::getArrayFromQueryString()), and twig will escape again. A quick fix would be to not escape in the template, but this would make the current strategy more entrenched. Still thinking about what the best fix is.

@diosmosis
Copy link
Member

I have a solution that will be a start for #4231.

@mnapoli
Copy link
Contributor

mnapoli commented May 28, 2015

Added to the (long) list in #6714. By the way @diosmosis do you think #6714 duplicates #4231? (e.g. same goals -> move to sanitizing on output) I could move information in the older issue if that's the case.

@diosmosis
Copy link
Member

Seems like there's a lot of overlap; The newer one has more information, a more definitive answer to the approach to take and you're obviously maintaining the list of stuff to do, so we could keep that one. We just need one in the 3.0 milestone I think.

@mnapoli
Copy link
Contributor

mnapoli commented May 29, 2015

Yours has more history, I've merged #6714 into #4231 with the information.

diosmosis pushed a commit that referenced this issue May 30, 2015
diosmosis pushed a commit that referenced this issue Jun 2, 2015
diosmosis pushed a commit that referenced this issue Jun 2, 2015
@mattab mattab modified the milestones: 3.0.0, 2.14.0 Jun 23, 2015
diosmosis pushed a commit that referenced this issue Jun 30, 2015
@SimonWaters
Copy link

Noted during testing this is still present in 3.0.0-rc3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants