Closed (won't fix)
Project:
Drupal core
Version:
8.6.x-dev
Component:
other
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
14 Feb 2015 at 18:03 UTC
Updated:
5 May 2019 at 10:01 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
star-szrSee what breaks for now.
Comment #2
star-szrComment #4
star-szrComment #6
star-szrComment #7
ParisLiakos commentedgreat, looks good and not that bigger..lets take it to tha main issue?
Comment #8
joelpittetComment #10
joelpittetComment #11
dawehner.
Comment #13
dawehner.j
Comment #14
dawehner.
Comment #16
dawehnerNext one
Comment #18
star-szrComment #19
star-szrComment #20
star-szrComment #21
manuel garcia commentedI've been testing this patch, and all seems in order. Malicious code is still escaped as expected, for example:
Tried entering
<script>alert('hacked');</script>as a class for a field, the result isclass="scriptalerthacked-script".I also tried a variety of other strings, everything is secured by twig, as expected.
I do have one question on this patch though, ran an interdiff with the latest patch on the main issue (#46), which you can find attached.
My question is on this part:
And:
Is this on preparation for a test or?
Comment #22
star-szr@Manuel Garcia where are you seeing those changes? Did you forget to rebase maybe? I'm not seeing those in the patch in #18 or #20.
Comment #23
manuel garcia commentedOh my god Cottser sorry... thats coming because I have another patch applied to fix the problem with devel_generate.. #2422101: CommentItem should override the generateSampleValue method and provide sample values... please ignore.
I think we are ready to put this into the main issue, will do that now, thanks!
Comment #24
star-szrComment #26
star-szrComment #27
star-szrComment #28
star-szrComment #29
dawehnerJust research.
Comment #31
star-szrInitial versions first.
Comment #32
star-szrAnd updated ones where applicable.
Comment #37
star-szrComment #38
star-szrComment #40
star-szrComment #42
star-szrInterdiff for the last one, note that this hacks vendor, see https://github.com/twigphp/Twig/issues/1811
Comment #43
star-szrShould be greener :)
Comment #47
star-szrShould fix one fail or so. But I think adding Twig extensions is broken still.
Comment #48
star-szrComment #53
star-szrThat's why I shouldn't patch past midnight in most cases.
Comment #54
star-szrWon't be green but should have better syntax ;)
Interdiff is from #47…
Comment #55
star-szrTurns out Drupal\system\Tests\Theme\TwigExtensionTest is not passing for me locally even on HEAD.
So just adding some other stuff here to play with, test extension shouldn't be setting a bad example extending core's extension.
Comment #60
star-szrComment #61
star-szrComment #62
star-szrYeah it doesn't help for running this test locally when you have a twig_extension_test.twig.test_extension service in your contrib modules folder from playing around months ago :(
Comment #66
star-szrGetting closer…
In theory all that's left is resolving Drupal\Tests\Core\Template\TwigExtensionTest that doesn't seem to like the 't' and 'trans' filters we have defined because
callableis now hinted and I guess it's not able to resolve 't' from bootstrap.inc because unit tests?Comment #69
star-szrDoesn't have the new cache class yet but it's all I have time for at the moment. This is 1.x.
Comment #70
star-szrOops wrong file.
Comment #72
star-szrComment #73
star-szrIt's good to learn things.
Comment #76
star-szrNo other changes just the upgrade to see what happens.
Comment #81
star-szrFaking the cache for now.
Comment #84
star-szrHmm forgot to include the new class.
Comment #87
star-szrComment #89
star-szrComment #90
star-szrComment #93
star-szrComment #94
star-szrThis just removes an unneeded (old) class TwigCache that I added earlier as a temporary measure.
Comment #95
star-szrRemoves changes from #2567139: Twig autoescape default strategy deprecation error triggered on every page load.
Comment #96
star-szrComment #97
star-szrComment #98
star-szrComment #107
star-szrComment #108
star-szrComment #109
star-szrComment #111
star-szrComment #113
star-szrComment #115
star-szrComment #117
star-szrComment #119
star-szrComment #124
star-szrComment #127
adamjuran commentedComment #128
star-szrComment #133
star-szrComment #136
star-szrComment #139
star-szrComment #142
star-szrComment #145
star-szrComment #148
star-szrComment #151
star-szrComment #153
star-szrComment #156
lauriiiAt least its a start
Comment #161
star-szrComment #169
dpiDead