Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Similar to #1938972: Start moving string functions into a utility class let's move filter_xss and all it dependencies into the String class.
Comment | File | Size | Author |
---|---|---|---|
#29 | drupal-1998466-28.patch | 31.42 KB | dawehner |
#29 | interdiff.txt | 5.09 KB | dawehner |
#27 | drupal-1998466-27.patch | 31.17 KB | dawehner |
#27 | interdiff.txt | 7.25 KB | dawehner |
#21 | drupal-1998466-21.patch | 27.99 KB | dawehner |
Comments
Comment #1
dawehnerThis one doesn't move the tests yet to PHPUnit but we could, if needed.
ParisLiakos suggested to inject the allowed protocols via a static method.
Comment #2
ParisLiakos CreditAttribution: ParisLiakos commentedi think we should not have the "drupal" word there, since you know this is a component, and could be used outside from Drupal and stuff
oops, should be splitted to some lines
needs identation
should use the class method
indentation again
oops:P
Comment #4
dawehnerLet's see whether this works better, but i'mt not optimistic.
Comment #6
ParisLiakos CreditAttribution: ParisLiakos commentedUncaught PHP Exception Drupal\\Core\\Database\\DatabaseExceptionWrapper: "SQLSTATE[42S02]: Base table or view not found: 1146 Table 'd8.simpletest65608router' doesn't exist: SELECT name, route FROM {router} WHERE pattern_outline IN (..)
during upgrade...this is...weird
Comment #7
ParisLiakos CreditAttribution: ParisLiakos commentedhere, that should fix it.
i think...we should move the allowed protocols stuff out of the string class
Comment #8
dawehnerLet's move the protocol specific bits to an URL helper class.
Comment #10
dawehnerAdding the new files would certainly help. Paris mentioned that validateUtf8 should be on Unicode.
Comment #12
dawehnerI'm wondering whether it's okay to call something on the URL class out of the String class, but if we can't do that filterXSS has to be part of something else.
Comment #13
ParisLiakos CreditAttribution: ParisLiakos commentedi dont see, why that would be wrong
this could use an @see Url::stripD...
This needs update
this should be Url::..seems there is no coverage there:/
this is really freaking ugly, but i understand that might be out of scope here, since we are just moving arounnd code
the first "to" should be removed
Comment #14
dawehnerWell, everything which called filter_xss_bad_protocol before got already converted to use the methods on the classes.
Comment #15
dawehnerJust in case someone (like paris) is wondering: _drupal_bootstrap_configuration() is before _drupal_bootstrap_kernel() so you don't have the config
available.
Comment #16
dawehnerLet's move it to _drupal_bootstrap_code()
Comment #17
dawehnerMoved from the String class to a Xss class.
Comment #19
ParisLiakos CreditAttribution: ParisLiakos commented#17: drupal-1998466-17.patch queued for re-testing.
Comment #20
ParisLiakos CreditAttribution: ParisLiakos commentedneed update to point to Xss class
no longer needed
Comment #21
dawehnerGood catch!
Comment #22
ParisLiakos CreditAttribution: ParisLiakos commentedready to go, thanks!
Comment #23
ParisLiakos CreditAttribution: ParisLiakos commentedComment #25
ParisLiakos CreditAttribution: ParisLiakos commented#21: drupal-1998466-21.patch queued for re-testing.
Comment #26
ParisLiakos CreditAttribution: ParisLiakos commentedsigh, entity translation module is infested with random failures:(
Comment #27
dawehnerRenamed Url to UrlValidator and put valid_url
Comment #28
damiankloip CreditAttribution: damiankloip commentedSeems like we could improve the param/variable names here, and go for $attribute etc.. maybe attribute_array....
Comment #29
dawehnerBetter variable name.
Comment #30
damiankloip CreditAttribution: damiankloip commentedThanks. That looks good now!!
Comment #31
alexpottCommitted 23b5912 and pushed to 8.x. Thanks!
Comment #32
dawehnerThanks for committing.
Added a follow up for the phpunit tests: #2006568: Convert filter_xss tests to unit tests