Not sure why I hadn't closed this yet.
This comes from auto_entitylabel_entity_presave(), which checks if the label is equal to the placeholder value, but if the label is NULL, then it won't do anything.
I think the previous behavior could be maintained if the check included NULL and the placeholder value.
I don't think I was correct about this, but there are tests now.
Was also running into a similar issue as @michèle in
#106
🐛
Set Label runs two times on node creation
Needs work
as title was calculated based on a saved value (via serial module) so "Before save" is not an option. I had a hook_node_insert implementation that checked isNew()
, which no longer passed because of this issue. Luckily I no longer needed to do that extra check.
I have custom code that creates a node and prior to the change I was able to not set a title value using the create method on the entity storage handler. It looks like if I provide the value of "%AutoEntityLabel%", then programmatically created node is successful without the integrity constraint violation exception.
This comes from auto_entitylabel_entity_presave(), which checks if the label is equal to the placeholder value, but if the label is NULL, then it won't do anything.
I think the previous behavior could be maintained if the check included NULL and the placeholder value.
Looks good locally and with test run.
Quick merge.
Should be resolved now.
This should now fail because of the PHP warning.
I'm satisfied with my testing locally.
Merged.
mradcliffe → created an issue.
I read CWE-863 and CAPEC-87 and those seem like the closest for the Information Disclosure vulnerability in SA-CONTRIB-2024-034 (freelinking row 42).
I looked through some of the children of the parent CWE-285 and CAPEC-115, but did not see any children that were more specific for Information Disclosure.
Should be good. Need to do some manual testing and let this gel for a bit.
mradcliffe → created an issue.
Also the test run from 3 months ago passes on PHPUnit 9:
PHPUnit 9.6.20 by Sebastian Bergmann and contributors.
.................................. 34 / 34 (100%)
Time: 00:02.545, Memory: 6.00 MB
But this will not be (backwards)compatible with PHPunit 9 I presume?
It should be easy to figure out by adding test previous major into gitlab CI yaml. I needed to add that anyway to most of my modules because of the flip to testing Drupal 11 as the current major.
Revoked Drupal 10 compatibility from 2.0.x branch because it is not possible to be compatible with Drupal 10 and Drupal 11 due to Symfony incompatibilities.
It is not possible to make this module compatible with both Drupal 10 and Drupal 11 thanks to Symfony version incompatibilities.
So I'm going to revoke 10 compatibility from 2.0.x.
Added some better alternative text to the typed data diagram.
og is only a dev dependency so setting it to ^1.x-dev for now.
I don't think this is RTBC, but I wanted to show red-green test runs.
mradcliffe → created an issue.
anybody → credited mradcliffe → .
I wasn't sure if fixing phpcs wold be in scope, but it's great to see fully green. Thank you, @liam morland.
Merging for both 2.0.x and 1.0.x.
mradcliffe → changed the visibility of the branch 3455705-refactor-tests-for to active.
mradcliffe → changed the visibility of the branch 3455705-refactor-tests-for to hidden.
Unpostponing, tests are running on D11 by default now on this branch.
That should do it. Tests pass for Drupal 10 (previous major) and won't pass for Drupal 11 until module is compatible with Drupal 11.
I added a test as well, but this will probably fail due to 📌 Automated Drupal 11 compatibility fixes for nodeinfo Needs review .
mradcliffe → created an issue.
I cleaned up the issue summary a little bit and added the branches to the issue summary.
10.3.x, 10.4.x and 10.5.x composer.lock files match previous release of composer so I cherry-picked the 10.3.x commit into branches for 10.4.x and 10.5.x.
I'm going to re-open this as Normal since there is a workaround, but we should still update the dependency in core.
I'm removing the performance and security tags since that is no longer relevant. And maybe it's probably a task now.
I ran the pipeline again on the merge request, and it passed.
Updating twig/twig caused some Functional failures so setting this to Needs work.
twig/twig:3.14.2 and twig/twig:3.11.3 are released now by Fabien.
This needs to be in 11.x-dev since it needs to go into 11.0.x, 11.1.x, 10.3.x and 10.4.x.
Adding some tags, updated issue summary with sections from template.
I don't feel like only checking \is_object($v) would be acceptable upstream since the point of the method ensureToStringAllowed() method checks arrays as well, but as soon as we recurse via arrays too, we run into the recursion limit.
A potential Drupal-only solution would be to create a custom DrupalSandboxExtension that copies the code in SandboxExtension, and maybe checks $policy::$allowed_methods somehow to ensure that nobody has overridden core's TwigSecurityPolicy to remove __toString as an allowed method. If it is still allowed, then we can bypass the call to that method entirely and return $obj, and if somehow someone is doing something funky in custom land, then it would try its best with the original code.
We cannot extend SandboxExtension because it is final so this would require maintaining the code separately.
And then maybe we can work with twig/twig:4.x to allow for a way to override ensureToStringAllowed on the policy level rather on the extension level so we can get rid of it.
I filed an issue upstream - https://github.com/twigphp/Twig/issues/4439
I think I spoke too soon about it being infinite in our case, but it does hit the recursion limit. Sorry.
Checking the is_object($v) helps to reduce that.
I don't know if this should be addressed upstream or not, @gillesbailleux. Reporting an issue upstream might help to get someone more knowledgeable about the right approach though.
Enabling xdebug provides a better stack trace.
Following Issue Priority > Critical > Bug → , I think this qualifies as a Critical bug report as the site becomes unusable when applying the release for content editors. Either render system or theme system.
# 3.14.1 (2024-11-06)
* [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects
They are now checked via the property policy
* Fix a security issue in the sandbox mode allowing an attacker to be able to call `toString()`
under some circumstances on an object even if the `__toString()` method is not allowed by the security policy
We may be doing something that's triggering this based on the BC BREAK note here.
I also ran into this updating my local ddev development environment from 10.3 to 11.0, but prior to updating to 11.0 I did run composer update -W per upgrade instructions.
I added the empty stracktrace from #8 that I also got into the issue summary.
This seems like a bug report.
mradcliffe → created an issue.
Confusingly, this can also happen if you are writing a new kernel test even without any request object being constructed when making a mistake with dependencies or services.
Made a little more progress. Done for now. Maybe some changes with membership since I did the PR originally?
This looks good to me.
Potentially setting core_version_requirement to ^10.3 || ^11 might help for the next stable release as well in case anyone one who hasn't gotten to 10.3 tries to update and runs tests. That could also be done in the other issue as well.
Thank you for going through this effort, but no further development is happening on this module due to the processing.js library being deprecated. The module must be completely written for a different library before any further development occurs.
It would probably be better to look for an existing issue within a contributed module to work on first.
I updated IEF to 3.0@rc so the test on next major runs, but there is a failure in D11 that's not present in D10. I'm not sure if it's related to IEF or not.
Seems good to me. Merged.
Could a site builder end up creating a situation where they set the sub title to use h2 and then the hierarchy no longer matches between teaser and page views?
- Page
- h1: Node Title via page-title
- h2: Sub title
- Teaser
- h1: Page title
- h2: Node title
- h2: Sub title
I added some items to the work needed based on Slack.
All green.
mradcliffe → created an issue.
Issue cleanup.
Sorry for the very late response.
I looked at the site_alert module and I think the only thing that may be causing the double render is that the module has a route it loads via JavaScript, /ajax/site_alert
.
If the agreement is configured with Visibility to "Show on every page except the listed pages", then adding that path to the Visibility Pages configuration will prevent the Agreement from rendering when that path is requested.
mradcliffe → created an issue.
We can start by creating an issue fork and merge request for 11.x, and the issue is in Needs work based on comments above. Once in Needs review, it might help to add this to the Needs Review Queue Initiative or discuss in an appropriate Slack channel.
I removed the beta table adjusting the issue summary to compensate because that was something we did specifically for Drupal 8 and no longer applies.
Removing Needs issue summary update tag and setting status to RTBC because I think recent comments were addressed.
Applied #13 patch and commit now that I should be unblocked on another issue. Leaving active because Drupal 11 compatibility depends on module dependencies that are not yet ready.
Yes, it's firefox ESR. If it's related to that, those will probably go away when I'm able to get on the next ESR release within the next several days.
Updating component to Trial Experience.
mradcliffe → created an issue.
Probably should have fixed this a long time ago.
Added a merge request. See what fails and address those items.
I removed the UX review task in the issue summary and added a note about it in the User interface changes section.
I added the Needs issue summary update tag for further updates to the User interface changes section to update the screenshot.
One of the most helpful things that @volkswagenchick created as part of the First-time Contributor Workshop slide deck were hidden slides at the top with instructions on how to modify the deck based time and audience as well as reminders about how to copy and share the slides with accessibility in mind.
These would be helpful as well for a shared deck.
Added mention for issue credit for PR review and work on GitHub for users mpp, semiaddict, voleger.
Okay, all migrated over. Probably several code quality issues to fix and tests to fix so bumping to Needs work.
mradcliffe → created an issue.
mradcliffe → created an issue.
mradcliffe → created an issue.
I forgot to double-check user names.
Alison is providing feedback at GovCon 2024.
Oh, you're trying to download the module from the fork? This is a composer issue rather than a git issue. The branch and issue fork exist in git.
But it would be easier to test the issue fork in DrupalPod there is a browser extension for DrupalPod that makes this seamless on an issue page.
When I'm working locally with my own modules, I have the module checked out directly and can add git remotes and checkout branches from issue forks.
quietone → credited mradcliffe → .
I was able to clone the fork and branch successfully on a new DrupalPod instance.
gitpod /workspace/drupalpod/repos/theme_switcher (3434980-gitlabci) $ git status
On branch 3434980-gitlabci
Your branch is up to date with 'theme_switcher-3434980/3434980-gitlabci'.
nothing to commit, working tree clean
gitpod /workspace/drupalpod/repos/theme_switcher (3434980-gitlabci) $ git remote -vvv
origin https://git.drupalcode.org/project/theme_switcher.git (fetch)
origin https://git.drupalcode.org/project/theme_switcher.git (push)
theme_switcher-3434980 https://git.drupalcode.org/issue/theme_switcher-3434980.git (fetch)
theme_switcher-3434980 https://git.drupalcode.org/issue/theme_switcher-3434980.git (push)
I'll merge origin/2.x and push to the fork. Hopefully that helps.
Re-postponing on gitlab ci.
Re-opening.
Drupal 11 is released. Closing as fixed.
mradcliffe → made their first commit to this issue’s fork.
Thank you for the patch. I reviewed it, and applied the .module fixes.
I created an issue fork branch with an initial commit, but I think it is Needs work because Lucas, Elli and Brian had similar questions about participating without being physically at events so I did not want to remove their names yet.
I also was not able to reproduce. This looks to have been resolved with 📌 Making a theme compatible with core's theme generator is too difficult Needs work in Drupal 10.3.x and higher.
More info on the change record: Starterkit themes now use starterkit.yml file → . The src/StarterKit.php file is ignored.