prudloff → created an issue.
The check_markup() doc says:
this function should only be used when filtering text for use elsewhere than on a rendered HTML page. If this is part of an HTML page, then a renderable array with a #type 'processed_text' element should be used instead of this
So I think we should use something like this:
'text' => [
'#type' => 'processed_text',
'#text' => $text,
'#format' => 'footnote',
],
'text_clean' => PlainTextOutput::renderFromHtml($text),
I don't think the format needs to be applied to text_clean since PlainTextOutput::renderFromHtml() removes every tag anyway.
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
I think an exception throw is the best way to notify the developer..
I agree but I wonder if it could be a breaking change if some custom code relies on passing something that is currently evaluated to true?
What's the point of having a second ResultPlural class parallel to the existing one?
This is explained in #56.
Not that both |raw and Markup bypass Drupal's XSS filter so the module should make sure the markup in this variable is safe: 🐛 XSS in footnote text Active
Is there any reason to truncate the hash in ImageStyle::getPathToken()?
Would using the whole hash cause problems?
prudloff → made their first commit to this issue’s fork.
A potential solution would be to add a warning for text format when user uses configuration that is known to enable self-XSS.
Could this be covered by ✨ Provide warning when trying to use dangerous HTML markup Needs work ?
prudloff → created an issue.
This module does something similar: https://www.drupal.org/project/protect_form_flood_control →
prudloff → created an issue.
I merged the latest 11.x
I merged the latest 11.x.
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
I feel like this issue is mixing two different performance problems:
- Issue summary: this module slows down the installation process because it generates a lot of SQL queries.
- Some comments: rendering the toolbar is slow (mostly because of access checks)
I think it should focus on the initial problem and a separate issue could be opened for the slow rendering.
prudloff → changed the visibility of the branch 11.x to hidden.
prudloff → created an issue.
In fact, an attacker could look at the code of security_review on drupal.org for ideas of things to check even if the module is not installed.
I think it is true for most of the checks, but some checks could reveal information that would not be available (or hard to acquire) without this module.
For example TemporaryFiles provides a list of sensitive files that could be downloaded. Without this information it could be possible to brute force the file name but if the file name is unusual it would be much harder.
It seems security_review has a similar check: https://git.drupalcode.org/project/security_review/-/blob/3.1.x/src/Plug...
Based on a list of unsafe tags: https://git.drupalcode.org/project/security_review/-/blob/87d5f3ea84b0be...
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
prudloff → made their first commit to this issue’s fork.
There is also a contrib module that does this: https://www.drupal.org/project/remove_reset_password →
The change could be similar to this: https://www.drupal.org/node/3359827 →
prudloff → created an issue.
Sorry, wrong status.
Note that other authentication methods (basic auth for example) could also be vulnerable but it might be impossible to support every authentication provider.
The module could display a warning when a method it does not support is enabled. Something similar to this: https://git.drupalcode.org/project/one_time_password/-/blob/37a3c61bc1ab...
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
I also had a look at how WordPress handles this: it is not possible to use a standard password to login with REST.
Users have to generate an application password that can then be used to login with the REST API.
These passwords are always long and randomly generated, this makes it more OK to not have 2FA on this login method because application passwords would be very hard to brute-force.
prudloff → created an issue.
I thought the changes to user_role_grant_permissions() were lost by mistake will working on the MR, but it turns out they were committed in 🐛 Config entity static cache is not cleared correctly when multiple language overrides are used Active : https://git.drupalcode.org/project/drupal/-/commit/30fb2bde8c479d9fd3be0...
prudloff → created an issue.
prudloff → changed the visibility of the branch 2741939-8.9.x to hidden.
prudloff → changed the visibility of the branch 2741939-9.2.x to hidden.
prudloff → changed the visibility of the branch 2741939-cannot-use-a to hidden.
I suppose we could wait for 📌 Allow big pipe to run for session-less users Active then test again to see if it works correctly with cache_control_override.
I just noticed that the purge module now adds more options to the form: https://git.drupalcode.org/project/purge/-/blob/e5ca166bff3c0573f2c84cab...
This covers our use case of needing to set a large value when using a reverse proxy (and I think most websites using a reverse proxy will use the purge module).
So I would be OK with closing as wontfix.
Not sure this was discussed yet: browser cache invalidation can be a challenge.
For files in the importmap, we can add a GET parameter (generated with AssetQueryString) to invalidate browser cache when the file changes:
"swiper": "\/themes\/custom\/front\/node_modules\/swiper\/swiper.mjs?sxoxj0"
However if this file then does another import with a relative path:
export {default as Virtual} from './virtual.mjs';
We have no way to invalidate browser cache when this other file changes.
Sorry I didn't know that assertion messages should not be used. (Maybe a phpcs rule delicould catch that?)
I am OK with your changes.
I created a MR from the latest patch.
This still needs some tests and there are some @todo in the code.
prudloff → made their first commit to this issue’s fork.
smustgrave suggested that we add a change record.
I rebased the MR.
The phpstan errors listed by the bot in #32 seem to be unrelated.
Tests are now green.
Removing "Needs issue summary update" because the summary was updated in #136.
prudloff → made their first commit to this issue’s fork.
prudloff → made their first commit to this issue’s fork.
prudloff → changed the visibility of the branch 3428552-automated-update-d11-stable-1.4 to hidden.
prudloff → changed the visibility of the branch project-update-bot-only to hidden.
I've reverted some of the code that removed datetime_wrapper theme hooks and preprocessing and I think we need to add the templates back as well and deprecate that properly.
I added the templates back and added a deprecation.
And I drafted a change record: https://www.drupal.org/node/3530128 →
prudloff → made their first commit to this issue’s fork.
I cleaned up and fixed the tests in the MR for the drupalSettings approach.
I had to change multiple selectors in tests because now tests using the stark theme don't have access to the messages--[type] class anymore (this class was always added by the JS but is not in the base status-messages.html.twig template.
If we decide to use the web component approach instead we should probably start a new MR.
Tests are now green.
Hiding patches to focus on the MR.
Tests are failing so we need to fix that before it can be reviewed.
Hiding patches to focus on the MR.
We now have 4 MRs on this issue, which one should be reviewed?
I opened a MR based on the latest patch.
I looked at every hash_equals() call in core and found only one place where the parameter order was wrong.
My rationale for adding this to core would be that almost every other machine name in Drupal can be customized from the UI.
It looks like it was broken by this change: https://git.drupalcode.org/project/obfuscate/-/commit/c8680a4edf71277cc3...
This means that we are going to double-encode these characters. For example data-title="Me & my friends" would become data-title="Me & my friends".
Note that this problem already exists in other places in core. For example when using (string) new Attribute(['data-title' => 'Me & my friends']);.
I rebased the MR.
I rebased the MR.
We also got bitten by this. We have a lot of slots without a title or description and this doc explicitly says it is valid: https://www.drupal.org/docs/develop/theming-drupal/using-single-director... →
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
I think comments have been addressed.
Removed patches to focus on the MR.
You don't need to create a new issue in that case, you can update the MR branch or create a new MR.
prudloff → changed the visibility of the branch 3443679-use-elementisempty-to to hidden.