🇺🇸United States @greggles

Thanks for the help testing and looking for what has changed and any help you can provide in fixing it. So it works with 10.2, but not 10.3?

And what about the 8.x-1.x branch - does that work with 10.3?

We may want to add a note to the project page until this is fixed.

@jcnventura - yes, thanks for creating that release. I'm not sure I understand why we should wait for Drupal 11 until making the 2.0.0 stable?

I just reviewed 3367638 and it looks good to me. Since I've got the last commit on it I'd love if you could review the changes I made?

Thanks!

Sorry for not responding sooner on the question in comment #7, but it looks like you got it all sorted. Thanks!

That new select query looks much more performant and scalable to me compared to the original query and user load approach.

I believe the IN condition has a practical limitation and may fail on a site that has many thousands of uids to delete. I tried a site with 150 users and it worked fine, but tens of thousands or hundreds of thousands would probably fail. That would be a pretty big site to have that many DELETED users.

I think a more scalable approach would be to do these as single deletes and (in my experience) the performance of running many delete queries with a single = condition this is not much slower than running one big IN query. I've pushed a commit with that change.

I also think the indentation in login_history_user_delete was a little incorrect so I tried to fix that, but I'm also using a freshly setup IDE so I might have made a mistake there.

Can you give that a review, @brooke_heaton?

2.x branch usage remains quite high and the only bug is 3367638 so I think it would be quite appropriate to make a new stable.

> Needs an update hook to modify existing installs with the new views definition.

I think it could be OK to leave old installs as configured. We could add to the release notes a suggestion to modify the view or revert or whatever if they need it.

Adding more help text is not necessariliy helpful, so I think we can close this. If someone has a proposal for help text that is helpful we can reopen this or have a new issue.

+1 to the proposal as a concept. I didn't review the code for completeness/effectiveness.

I think this should maybe go into 2.x first and then be backported to 8.x-1.x.

Thanks for proposing those, those are great things to highlight and get included.

Since the current stable is quite old and several fixes are in dev I suggest the best action is to make a stable with the current code. Then those 4 issues can be reviewed and added to the dev branch and a new stable could be created in a few weeks or few months.

Can you try 8.x-2.x-dev release?

I think the changes in 🐛 Exception difficult to debug without message Fixed will make it possible to debug this issue.

Then this becomes another reason to do the release mentioned in 📌 Create new stable release for 8.x-2.6 Active .

I created 📌 Create new stable release for 8.x-2.6 Active which would get these instructions in a stable release.

greggles → created an issue.

Thanks for providing the MR and bug report.

Can you provide more info on the step 3 or what the expected vs. unexpected results are in the steps to reproduce?

Thanks!

VladimirAus → credited greggles → .

xjm → credited greggles → .

Is this for "Git vetted user" or membership in (and the role) of "security team" ?

I think this is an important risk for us to manage, but agree with dww we don't want undue burdens in the way of contributing.

poker10 → credited greggles → .

poker10 → credited greggles → .

xjm → credited greggles → .

greggles → credited greggles → .

If you aren't greggles, please leave this issue alone. Thanks!

greggles → created an issue.

Thanks for this proposal, @cedewey and for making the fixes, @Diwakar07!

@cedewey are you able to review the proposed merge request?

OK, I guess we should mark this as a duplicate of that one. I've given credit to stefvanlooveren over there since this issue predates that one.

@daveferrarra1 would you consider making your patch into changes in the MR?

Adding credit for stefvanlooveren who proposed part of the fix here before this issue was created over in 🐛 Error: Call to undefined method Drupal\Core\File\FileSystem::uriScheme() RTBC . I'm marking that as a duplicate of this one since this has more folks and fixes more issues.

This looks good to me. I see the change record at https://www.drupal.org/node/3201242 →

@christophe.klein I see you assigned this to yourself. Is there something you were planning to do?

I visually compared https://git.drupalcode.org/project/paragraphs_browser/-/commits/8.x-1.1?... and https://git.drupalcode.org/project/paragraphs_browser/-/commits/8.x-1.x/... and they appear to contain all the same commits.

That's OK! It had been closed so long it was hidden from the default view. Your report helped me realize it was time for a new release, so I created a new release today. If you notice any problems please do keep reporting issues about them. Thanks.

OK, I did that https://www.drupal.org/project/paragraphs_browser/releases/8.x-1.2 →

OK, I guess this is a duplicate of that and we should make a release. I filed 📌 Create new 8.x-1.2 stable release Active to track that.

greggles → created an issue.

Can you test this out with 8.x-1.x-dev and see if it's still needed?

I think this was solved in a slightly different way in 🐛 PHP 8.2 Deprecated function: Creation of dynamic property RTBC .

Moving to "needs work" since the $uuid should get a docblock and a variable type (i.e. string, I guess).

It seems this issue might be responsible for a regression in 🐛 Regression since 2.1.2 - cache rebuild / markup cache disabling required to get module working Active .

Adjusting my credit - thanks.

I think this should be marked as "Closed - works as designed".

hestenet → credited greggles → .

Thanks for that contirbution! Does the original patch contribution by Nikhil_110 look right to you?

I have some concerns about scalability of the solution. The first query on 112 would load all the records for all uids, even if there are multiple records (right?). And then I believe doing an entity load of the user object to test if the user exists can be pretty resource intense on both the database queries and the RAM of the data.

What do you think about making the first query a left join from login_history to the users table that looks for cases where users.uid is null? That query could also be made distinct to limit the trips through the loop deleting records. Then there's no need to test if the user load succeeds.

Clarifying language on release timing to focus on the most common case. updating links from HTTP to HTTPS.

I believe this should only need to focus on .htaccess since 🌱 [policy, no patch] Drop support for Windows in production Needs review has been decided.

The research is SUPER helpful. Thanks paulmckibben!

I wonder if you could do `ddev drush cr` right after the config-set (perhaps scripted) to get things working?

The title here states this is a regression after 2.1.2. Is it a regression for you, @paulmckibben? If you switch back to the 2.1.2 version does the problem go away?

Thanks for those details, @paulmckibben! I didn't try to reproduce just yet.

I wonder what your process is for enabling stage_file_proxy in these development environments?

For me I have hooks in .ddev/config.yaml:

hooks:
    post-import-db:
        - exec: ./vendor/bin/robo run:update-database
        - exec: ./vendor/bin/robo run:enable-dev-modules 1

Those Robo tasks enable stage_file_proxy and run a cache rebuild. I'm not sure what can be fixed in an appropriate way in stage_file_proxy to do a cache rebuild, but I'm open to ideas of how to fix this if as a "bug" in stage file proxy if there's a proposal.

Version 2.1.4 on core 10.2.3 works fine for me with my settings.

For folks encountering problems can you provide more details like what your settings are?

If there is any error, what error do you experience?

@milosr can you provide more information on your setup? Please provide all settings on /admin/config/system/stage_file_proxy (please replace your origin hostname with example.com).

benjifisher → credited greggles → .

I think this is an interdiff of the changes from 16 up through #20.

Looks like some great improvements to me. Thanks!

YesCT → credited greggles → .

Also super appreciate you taking a look at this and committing it!

@jsacksick do you have an interdiff of your changes?

@mglaman -

Sorry, I thought it had explained it more on that page.

Which page?

I agree with the issue summary as written. It would be nice if Google could document this program more to confirm the behavior for some of the unknowns. Since you're in contact with them I assume you're in a good position to negotiate that.

@japerry this is the second time (between the private issue and this public issue) where you have closed things out prematurely. Please let the conversation unfold before adjusting status.

Your module users are confused in a way that makes them think your module is malware. That should be a huge warning sign that something in the module can be improved to reduce that concern and instead you cavalierly close it out with a dismissive comment? That is not appropriate collaborative behavior.

It seems good to document this in the README or in the code. Right?

I tested out a few combinations and the calculations all were exactly the same between the 2 calculators.

It looks like the form was updated to retain the input values, which is handy.

I guess the old page could have a redirect that sends people to the new page in case there are links to it. I looked for links inside of s.d.o and found none. I found one doc page link that will need to be updated and put that in the issue summary. I think that could be done now or once there's a little more testing.

smustgrave → credited greggles → .

I recently added the .well-known/apple-developer-merchantid-domain-association to a site and ran into this issue while troubleshooting that process. I agree with #3 that this change doesn't seem necessary, so I'm adjusting the status.

If there's no feedback explaining why this change is necessary within a few weeks I think the issue could be closed.

That seems OK to me. For what it's worth, the Webform Autosave → module was built to try to address this problem using optimistic locking. That feature has required careful tuning over the years.

IIRC the D.O. UI requires all projects when converting from 8.x-a.b to increase the major. 8.x-a.b is published by the packaging server as a.b.0. A collision could/would occur if the major jump was not enforced.

Ah, that's good to know. So that limitation (which I agree has good logic behind it) eliminates the situation I mentioned might cause a problem.

From the comments so far it seems that things are OK as they currently work.

I think we could consider this "works as designed" or if it's a "support request" phrased as a question then it could be marked "fixed".

From the screenshot: I think the classes on that release show it is not recommended (yellow background) but is security-covered (has a shield icon).

Semver doesn't seem to make any indications about security. Is that right? I think that makes sense. Semver helps communicate stability and the size/nature of changes, not the security.

In this case the project could opt out of security coverage and that would be an OK way to communicate it is not (yet) covered by security advisories.

If a project exists that has an 8.x-y.z release that is intended to be covered by security AND has a new 0.x.y release that the maintainer does not want covered by security that would require some rethinking of the logic of Drupal.org's code.

I don't have a strong feeling of what to do here, just wanted to share some thoughts I had on the subject.

The ideas in the OP still make sense to me, but I guess some of them will need to migrate to other locations.

Especially expiring passwords on accounts that haven't logged in for +4 years makes sense given our site's usage profile.

It has now been 14 days since the issue was opened and nicxvan contacted cburschka so I think this can be moved back to active.

I think you should currently have the ability to change branches and make releases. I'm happy to hop on a zoom to test it out if you want.

I followed this process for False Account Detector.

The old advisory was a mega-combined item from 2010: https://www.drupal.org/forum/newsletters/security-advisories-for-contrib... →

I added <del> tags around the old text and added this text:
<strong>Edited March 27, 2024:</strong> Previous versions of False Account Detector for Drupal core versions 5.x and 6.x contained these security vulnerabilities. The code has been rewritten completely for Drupal 10+ and is now available again.

greggles → credited greggles → .

Thanks for that review work!

Since there's a patch it should be in needs review status.

@phannphong do you feel this patch and the D10 Claro path work well together?

greggles → created an issue.

re #67, that is ✨ Provide documentation/default server block for Nginx server. Needs work . It's an interesting question and I think could help us decide that we should cut IIS and windows support to help us focus on more popular platform combinations.

I don't think that's quite it, but that would be helpful.

I updated the issue summary to remove an outdated remaining task and add some option items from comment 176.

Thanks for reporting the issue. It will help anyone else using the module who gets this same error.

I wonder if you start from a fresh test site if you are able to recreate the issue? I don't get this error on a site where I use paragraphs browser, so I wonder if there is some conflict with another module or a specific version of php or something that is causing it.

I see commits going in addition to the comment #2, that's maintenance.

The best way to help is to review things and propose patches.

Great work, smustgrave! I appreciate all your effort to improve this module and modernize it.

Yes, totally agreed. Sorry my brevity made my question confusing.

I've updated the page now. Leaving as needs review for a bit for your feedback and any other maintainers.

Thanks, nicxvan!

Is role based a good feature in your opinion? I think so, just want to be sure.

nicxvan → credited greggles → .

As you noticed, some "smartquotes" and missing https://www were causing this problem. Fixed now. Thanks for reporting it.

mcdruid → credited greggles → .

OK, this is now done. I've reviewed and made a pass at improving all the templates for current processes and standards (e.g. https and www on urls). I deleted 3 pages that seemed unlikely to be used and were therefore making it harder to find the message people do want.

Here's the content of the 3 I deleted so we can restore if someone wants them:

1750922 Title: Follow-up with maintainer about plan for module releases

Subject: [Drupal Security] Upcoming release for your module

Dear {name},

We have reviewed the patches for the module and everything is in place to release on this coming Wednesday {specific date}. The security team meeting starts Wednesday at noon (Eastern time USA). You must commit the fixes and create new release(s) of your module no later than Wednesday at noon. You must include 'Security update' when selecting the release type. In order to make the coordination easier, you may commit the fix and make the releases up to 24 hours in advance - in other words, any time after noon (Eastern time USA) on Tuesday. As soon as you make the releases, please update this issue with the URL's to the releases.

If you have not done so already, please create a draft SA at httpS://security.drupal.org/node/add/advisory . You can base this text on an existing SA from http://drupal.org/security/contrib

Sincerely,

1750880 title: Reply to request for Drupal.org maintenance

The security team deals with security issues in the Drupal code itself. The management of the Drupal.org website is not handled by the security team, but rather by the Drupal.org site moderators team. You can let them know what you need done by creating a new issue at the following URL: http://drupal.org/node/add/project-issue/site_moderators.

1750930 title: Reply to request for new service

Hello,

Thanks for your help to make other Drupal sites secure and for your suggestion.

The Drupal Security Team focuses its limited volunteer time on handling issues in Drupal core and contributed code hosted on drupal.org. We appreciate your idea but do not have the resources to make it a priority relative to our other tasks.

[Any issue specific notes like suggesting the Security Review module http://drupal.org/project/security_review]

You are welcome to move forward with this idea if you have the time.

Regards,
[Your name] on behalf of the Drupal Security Team

forgot to adjust credit

I cleaned up these:

https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... → - this one required removing a section that was an outdated concept about using the security update tag on non-stable releases.
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →

language cleanup

removing outdated idea about security update tag

language cleanup

language cleanup

langauge and usability cleanups

langauge and usability cleanups

poker10 → credited greggles → .

greggles → credited greggles → .

greggles → credited greggles → .

poker10 → credited greggles → .

Updating credit for this work.

I worked on

https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →
https://www.drupal.org/drupal-security-team/security-team-procedures/sec... →

Also noted some in the issue summary that I think should be deleted as they are no longer relevant.

language clean up

language cleanup

language cleanup

language cleanup

language cleanup