I will create a new module that is no problem.
I can speak to the rationale for avoiding "Version disclosure" in the project page. That is documented a bunch of places. But what should I say for the choice made here to preserve version disclosure as a feature, not a bug. Is it really just "It is occasionally helpful to developers to know which version of a JS library is in use." or is there a better reason?
This is advertising the version, not obscuring it.
Sorry for the confusion. I meant that it only shows "Drupal 10" and not "10.3.7" it only shows major version, not specific version.
These are all good solutions,
- using the patch from this issue
- deleting intall.php and update.php with aggregation on prod environments
- adding custom hook js alter.
any of which are no problem for me, a semi-experienced developer to implement. I am not sure if that holds true for the thousands of Drupal CMS adopters coming down the river.
I appreciate that nod_ thank you. One last way to think about this, my final pitch I promise ;)
As longwave points out, Security through obscurity is not the best... by itself. It is the equivalent of me hiding my house key under the flower pot, but it is still better than leaving it in the door.
An old timey phrase of "Loose lips sink ships", or the more modern "Snitches get stiches" seem to apply.
What if the shoe was on the other foot?
Drupal intentionally obscures its own version
``
What if some third party library decided to output "Drupal Version 10.36" because running internally it can detect the version. Would we want some other library disclosing that for us. Some might even find it helpful for development. I am pretty sure that leak would get closed in a hurry.
Why is our own obscurity fine, but we don't show the same care with third party code.
Drupal is popular in government, but so is this particular scanner. The amount of time people like me spend responding to the monthly scans is more than I enjoy. The "higher ups" don't always understand enough to say "this finding is legit", or "this finding is silly" They pay for the service and they want to trust its findings. Then I have to say, "it is not really anything to worry about"... then they search for "version disclosure" and they find pages like this and pretty soon they are starting to second guess my professionalism, because I told them, it was no big deal. I'd prefer if the product I love (Drupal) did not put me in that situation.
Additionally it seems it's only concerned about jQuery version specifically.
This is not specific to Jquery. The scanner triggers on any third party library where a version is displayed. For any given month there would be ~5 libraries it would include in its findings. I use the patch from this issue and they all go away.
@nod_ That is a good concern. I grepped the Drupal core codebase for
- $_GET['v'] - no results
- $_GET["v"] - no results
I also examined the 22 instances of 'parse_str(' to see if anything was looking for a query param of 'v' . Found nothing
Same for the two instances of 'PHP_URL_QUERY'
I believe that nothing other than our tests (already altered for this merge request) are looking for a 'v' query parameter. My belief is based on the fact that ?v= does not exists with aggregation turned on other than a few special pages like update.php and install.php and they are not using the ?v param.
Bumping this up to critical because the module is basically unusable for anyone building sites with composer.
Nice. Thank you for handling this plopesc.
Yah it is probably more important that the content of the feed respects visibility permissions than the feed itself having its own visibility perms. That makes sense.
Thanks for the background and options. I wonder if there is no pubDate we could provide a fallback by grabbing a reasonable date from some other source like 'modified' or entity->updated, because I do like the concept of having the rss sorted by date.
Thanks. I was not using the dev. Composer pulled the latest tag, but i was looking in the repo at the block code, so that explains the discrepancy.
Nice improvement to the group name.
This does result in a secondary warning
Deprecated function: DateTime::__construct(): Passing null to parameter #1 ($datetime) of type string is deprecated in Drupal\dkan_rss\DkanRssCreator->dateSort() (line 287 of modules/contrib/dkan_rss/src/DkanRssCreator.php).
but that is just the result of having no pubDate
I think the Merge request !12122 for 11.x is ready for review. See QA steps in previous comment.
Thanks @mortona2k. I will take a look at the field compare module.
I had reached out to the X-ray audit maintainers a couple years back but they were not interested in combining projects.
I am going to close this now that we have an epic for this undertaking 🌱 EPIC: Merge Module Usage project Active
Thanks again mortona2k for the idea.
@cobblestone.consulting that is great news. I think it would improve both modules considerably.
I put together an epic as a place to do some planning 🌱 EPIC: Merge Module Usage project Active and refining the approach.
I can likely take on the bulk of work in July. Mainly I just want to start the planning and discussion now so that everyone involved is happy with the outcome.
Released with 1.0.36 →
Released with 1.0.36 →
This is a great solution @kul.pratap. Thank you so much for the contribution. And thanks @mortona2k for reporting it. I will get a release tagged right away.
I think this is where we have to handle rendering of the SDC of the individual alert entries
https://git.drupalcode.org/project/uswds_alert/-/blob/1.0.x/src/Controll...
This is the JS that needs to be replicated https://git.drupalcode.org/project/sitewide_alert/-/blob/2.x/js/init.js?...
The way it works is that the block is just a div target, line 2 in the init.js targets that div and inserts the contents of the alerts found in the endpoint. Our catch is that we have three different targets, so our version of init.js will need to figure out from each item in the endpoint which div it needs to get thrown into.
This is fixed. We now have three distinct blocks.
This is complete.
This is fantastic dmundra, thank you for this. It is a great addition to development on this module.
This is as complete as can be until we work on the theming and replicating the javascript.
I also updated the home page with the new version of the Readme.
This block as it exists is not yet using the new data endpoint.
I have the block existing, and the new endpoint that has all the data we need.
I am going to bypass the AC of "Block displays only sitewide alerts of style USDS Site Alert" at this time because all of the filtering and placing of alerts happens in the javascript and I think it will be easier to handle modifying the javascript logic once we have all three blocks.
- technically the page visibility isnt required any more as block setting can handle that
I don't agree with this. There is a big difference between the block level page targeting and the individual alert level page targetting. In the several places I am using this module, we need specific alerts to show on specific paged even if they are all the same type of alert.
I do like the idea of having types, but then you also need multiple blocks for being able to place different types in different regions.
If you have the perm shut off for anonymous users, the site will likely try to hit the endpoint to get the data for the alerts, but will not have access to it.
There are likely two solutions:
- If everyone should be able to see the alert, make sure you grant "view published sitewide alert entities" to anonymous users.
- If anon users should not see the alerts, then make sure the block perms are not set to show to anon users.
timozura → credited swirt → .
Thanks @mortona2k. I do like the features of that module. I had not seen it before. I reached out to the maintainer to see if they have interest in unifying the two.
🌱
Consider merging with Content Model and Site Documentation module
Active
Downgrading this to minor. It may be more trouble than it is worth.
Thanks for reporting this mortona2k and for looking into the cause.
The schema issues for the View config have been repaired.
Rather than fix the phpcs issues in the event subscriber, I removed it since we won't be firing this on an event, we will be using field or entity validation to trigger the validation.
On other modules I have received updates after the initial round. I think it is related to either of two things
1. Improvements made to the update bot / rector rather than newly announced deprivations.
2. Inadvertantly adding new code to the module that actually contains a deprecation.
Yes the credit awarding is an unfortunate casualty, but it is only a delay, as the this ticket can be closed like the D10 update ticket was once we are firmly in D11 and getting updates to D12. The community understands that credit is not always immediate.
The directions are pretty clear at the top of the ticket, but I agree it would be nice if it spelled out when it is finally safe to close.
Thank you so much @Nidhi27 for this contribution.
Fantastic work Nidhi27. This works great. Thank you so much for this contribution.
Functionally, this seems to be working. I can tackle the linting and phpunit issues over the weekend.
Closing this as it is a duplicate of another ticket that was already completed.
Setting this back to Active so it will continue to get any automated updates
Accept automated changes until this issue is closed
If this issue is left open (status of Active, Needs review, Needs work or Reviewed and tested by the community) and the "ProjectUpdateBotD11" tag is left on this issue, new changes will be posted periodically if new deprecation fixes are needed.
This is looking fantastic. Thank you @adriancooke
Probably want to force the module weight to make sure this fires AFTER sitewide_alert
https://www.drupal.org/docs/develop/creating-modules/understanding-hooks... →
Looks like maybe we could just implement this hook and then unset $page_top['sitewide_alert']
That lives in the base module. Can we override it by putting it in our module. ( I would not think so, but I have no proof)