🇧🇪Belgium @mr.baileys

mr.baileys → created an issue.

mr.baileys → created an issue.

Merged and fixed.

Thanks, merged.

mr.baileys → made their first commit to this issue’s fork.

Nothing left to commit, so marking as fixed.

Thanks, merged in 1.x

mr.baileys → made their first commit to this issue’s fork.

mr.baileys → made their first commit to this issue’s fork.

@rajab natshah: I opened it as a public issue since it requires "administer content types", and thus is not considered a security vulnerability as per https://www.drupal.org/drupal-security-team/security-advisory-process-an... →

mr.baileys → created an issue.

Changed the config schema type for type_description and existing_nodes_link_text from string to label. Not sure if we also want to make the thumbnail and icon paths translatable?

mr.baileys → made their first commit to this issue’s fork.

pfrilling → credited mr.baileys → .

poker10 → credited mr.baileys → .

mr.baileys → credited mr.baileys → .

Added _csrf_token to the affected routes, and fixed a test. Still expecting a test failure on "monitoring/sensors/force/test_sensor" (I don't think there is a button or link to force a single sensor I can trigger in the interface for easy testing)

mr.baileys → created an issue.

yesct → credited mr.baileys → .

As @erikbj also mentions in #131, the official proper way to handle headers with too many values is indeed to split them up into separate lines but with the same header name (see https://www.rfc-editor.org/rfc/rfc9110.html#section-5.2). This would also ensure it remains easy to implement downstream logic basic on the header ("if header contains xyz" is a lot easier than "if header-1 contains xyz or header-2 contains xyz or header-3 contains xyz ...")

So I think splitting the headers but keeping the name is preferable over the current solution appending '-{delta}'. One caveat however: the different header line values are combined with ",", which means the current implementation would have to be converted to generate a comma-separated list instead of a space-separated list (which in turn is not backwards-compatible). On the other hand, this is meant to be a debugging tool, so I don't think this is that big a deal.

@rgpublic: I understand your concerns, consider that code a poc (I think this might also be a candidate to be handled in a follow-up issue)

A check along these lines would be way better IMHO. It avoids the problems mentioned above and is really a functional test that will make absolutely sure that scripts are not executed.

Would there be a risk that the JS-test gives a false positive: embedded scripts blocked due to browser/browser config on the administrator's machine, while not blocked for other contexts/browsers/users? (compared to checking for the header, which guarantees that the change made in this issue works across the board)

2. What about sites which use nginx or another server... what would be neat is something on the status report that confirms this protection is working. That way we could point to documentation on how to set this up.

I added a hook_requirements-check to test for the presence of a CSP-header on public SVG-files. I'm assuming that if there is a CSP-header, the site owner knows what they are doing, if it is missing we issue the warning. We could make it more strict and check the contents of the header if necessary.

Currently linking to the change record for more information.

I added an update hook (system_update_11103) to update existing .htaccess files.

We'd need the web.config to support Windows servers. This is in scope for this issue, I guess.

If this is D11 only, then we are no longer shipping web.config → , so no changes required unless we backport to D10.

mr.baileys → credited mr.baileys → .

mr.baileys → credited mr.baileys → .

Typo

Information about the coordinated/reported/fixed section in SA's for unsupported projects.

Added instruction about marking release nodes as insecure.

Updating the instructions for unsupporting projects.

greggles → credited mr.baileys → .

mr.baileys → credited mr.baileys → .

mr.baileys → credited mr.baileys → .

mr.baileys → credited mr.baileys → .

damienmckenna → credited mr.baileys → .

mr.baileys → made their first commit to this issue’s fork.

greggles → credited mr.baileys → .

mr.baileys → credited mr.baileys → .

mr.baileys → made their first commit to this issue’s fork.

@xmacinfo: for public files, I think the only way to add the header is through via web server directives (MR here includes a change to .htaccess for this reason)

poker10 → credited mr.baileys → .

Re-titling, since the scope of this issue has shifted to adding a content security policy header, rather than sanitizing the file.

@rgpublic: makes sense, I added a condition so that the default CSP header is only set if none has been set earlier. I think it's up to CSP and other contrib modules to alter/set handler priorities?

* Converted the patch to a MR, rerolled for 11.x
* Added a test to confirm the presence of the CSP header for file downloads.

I'm not sure we should be adding the CSP header (Content-Security-Policy: "default-src 'none') for none image/svg+xml downloads, as that seems out of scope for this issue and might break existing functionality.

mr.baileys → made their first commit to this issue’s fork.

mr.baileys → credited mr.baileys → .

mr.baileys → created an issue.

mr.baileys → created an issue.

• Created a new branch issue-3249970-11x, cherry-picked the commits from @duadua and brought it up to date with 11.x;
• Fixed PHPCS violations;
• Tested with setting an environment variable via ddev/config.yaml, verifying it is used after this patch is applied;

mr.baileys → changed the visibility of the branch 3249970-support-setting-service to hidden.

mr.baileys → changed the visibility of the branch issue-3249970--runtime to hidden.

We should be careful when removing this, since some contrib modules seem to rely on this property being set (so a bit of an undocumented API?). See for example https://www.drupal.org/project/logintoboggan/issues/1165126 → .

Not sure if this was ever addressed as part of another issue, but closed as outdated, there should not be any D6 to D7 migrations at this point.

pameeela → credited mr.baileys → .

Agreed with @Anybody that current behavior when saving a translation of a media entity still is buggy. However, since in this IS there is no mention of translations, I think this issue should be closed as a duplicate of 🐛 Mapped media fields are overridden with metadata on translation save Needs review , which addressed fields being overwritten when saving a translation of a media entity.

BramDriesen → credited mr.baileys → .

mr.baileys → created an issue.

borisson_ → credited mr.baileys → .

Marking as duplicate of 🐛 RenderArray not found in inline entity form render Needs review since that one has progressed further.

Nevermind, the issue I had was related to 🐛 Unexpected language prefixes on sitemap index Fixed

mr.baileys → created an issue.

mr.baileys → created an issue.

I addressed the issues raised. I don't have a lot of experience with working on drupal.org issues via merge requests, so please do let me know if made a mistake.

I do think the previous point of feedback about using the correct translation of empty is valid.

I don't think this is the case: the callback is invoked when a user clicks on the "star"-icon to add a shortcut, so it only seems to make sense to add the shortcut in the active interface language.

I changed the translation source to "(Empty)" (which is already present in the core translation set.

Leaving to needs review for other opinions, but the patch in #24 only addresses shortcuts created after applying the patch. Since shortcuts without titles are still stored in the database, the deprecation notices will still appear on the shortcut management page. I think a complete solution either requires an update hook, or hiding the issue (as was done in the initial patches in this thread).

Using '(empty)' is IMO an elegant solution, but should we account for languages other than English?

Fixed code style issues.

I agree with @ibonelli in #16. The issue occurs when displaying or manipulating shortcuts that do not have their title set. This should not be possible, since 'title' is a required field on the Shortcut entity. However it is currently possible to create shortcut entities with NULL as title. The proposed solution hides the issue but does not tackle the root cause (there should not be any shortcuts with NULL as title.).

The easiest way to reproduce the bug is using the steps outlined by @loze in #10 (creating a page display on a view without a title, and then clicking the shortcut quick add link on that page), but even without Views the issue can occur on any page without a title. It does seem limited to Shortcuts added via the "quick add"-link, as the "formal" interface for adding shortcuts has the title field as a required field.

Test-only patch attached that triggers the deprecation notice.

I see a couple of options to resolve this issue:

1. Hide the "quick add link" on pages without a title. It does not make sense to add these pages as shortcuts, since without a title the link(s) don't seem to make sense to end users. This would be confusing to end-users though (why can't I add a shortcut to this page?)
2. Show the "quick add link" on those pages, but instead redirect it to the shortcut add form so users can provide a title manually.
3. Provide a default when adding a shortcut to pages without a title, for example a blank string or "(empty)". Write an update hook to replace the NULLs with the chosen default (easy if we go with a blank string, might be trickier when we choose a translatable string).

To fully address the issue, we will also need an update hook.

Setting to NR for the testbot.

Test failure was unrelated to this change, and tests are back to green, so reverting to RTBC.

Tried to rebase but ran into too many issues, so I cherry-picked the commits and created a new MR. I have updated the CorsIntegrationTest: the original test in this issue was written against asm89/stack-cors 1.x, while 11.x uses 2.x. The latter no longer returns http status code 403 for disallowed origins (see #3128982: Upgrade asm89/stack-cors to ^2.0 to fix cacheability → ).

Adding an additional index on the hostname-column does make sense. I ran a test with 100k records in the {honeypot_user}-table: the query executed in \Drupal\honeypot\HoneypotService::getTimeLimit() becomes 35x faster (from 0.073 seconds to 0.002 seconds) for anonymous users.

In our case, the slave replication showed "The slave is applying a ROW event on behalf of a DELETE statement on table honeypot_user and is currently taking a considerable amount of time (61 seconds)."

The only condition used when deleting rows in Honeypot is "timestamp < _expire_limit_", so adding an index on the hostname column will not fix that issue (maintaining an additional index might even make it worse.)

Original issue contains too little information to go on, and there has been no activity in this issue in over a year with no indication that this is caused by a bug in Drupal core, so closing.

There is not enough information in the original issue to determine what the cause of the 403 is, but this is a support request rather than a bug. Closing since this is a support request, there has not been any activity in over a year and there is no indication pointing to a bug in core.

1. +++ b/core/assets/scaffold/files/default.services.yml
   @@ -160,15 +160,33 @@ parameters:
   +    # Setting exposedHeaders: ['*'] will result in a Access-Control-Expose-Headers:
   +    # * response header. See
   

   This line should wrap at 80 characters. This will also fix the fact that header name and value are currently split over 2 lines.

2. +++ b/core/assets/scaffold/files/default.services.yml
   @@ -160,15 +160,33 @@ parameters:
   +    # Setting Access-Control-Max-Age header value to '0' or false will omit this from the response.
   +    # However, setting it to '-1' will explicitly disable caching. For example, 600 to
   

   Line wrapping should be fixed.

It might make sense to merge 🐛 Improve documentation for cors configuration. Needs work and this issue, since that issue is about adding and documenting the new allowedOriginsPatterns-option to cors.config in default.services.yml.

mr.baileys → created an issue. See original summary → .