You are my hero. Thank you for digging in and figuring this out @dmundra
Released as 8.x-1.19 →
Released as 8.x-1.19 →
Thank you vlyalko for this contribution. It is much appreciated.
paraderojether interesting find related to a discrepancy between gitlab ci and local run. I am not sure of what the difference is. I want to investigate this discrepancy further So I will leave the failed checks that you found in place as a way to test whether I have got gitlab CI matching full phpcs.
@nidhi27 as always, thank you for your work. Sorry I let this one get stalled out for so long.
Thank you again @adriancooke and @kelsmith. I think most of this design has been implemented and some refinements made in another ticket that took into account the rule add / edit method that did not exist when we were hashing things out. So I am going to close this out. And thank you arijit acharya for getting us off to a good start.
This is working fine in tugboat but for some reason in local ddev contrib instance the tabs do not show up. I think it is an env issue.
swirt → created an issue.
Assuming that the submodules should not be enabled, this seems to working fine and RTBC
Crediting tsquare212 for solving this with rules entity.
Crediting tsquared212 for the rules implementation that pulled this off.
This is already handled by the cleaverness of the rules. Crediting Tsquare212 for getting it accompished.
Closing this as this is already accounted for with the rules.
I am calling this functionally complete. The validation of Rules works. The messages need some UX improvement but the functionality works. This even has test coverage for the rules.
Fantastic work Jordan.wood Thank you so much for this contribution.
Note, the classes should have been added as attributes to the render array, however at this time, SDC do not support attributes
✨
Process #attributes render property
Active
This is largely complete. It did result in two more tickets.
- 🐛 Add html preprocessing for the message. Active
- 🐛 Get CSS/scss working for SDC Active
Tugboat build and linting is both running and passing.
After this is merged, then the magic button on the project page can be clicked to bring in the Readme.
Merging this is blocked by 🐛 Composer requirements do not exist Active . After that is merged, then retriggering tugboat and gitlab CI on this PR should pass.
Thank you rkoller for this suggestion and the accompanying documentation. I will work on this soon.
This is fixed as it was resolved by ✨ Add tugboat previews Active
Added to README and project page.
Crediting Fen for his suggestion to add this.
Closing this out with the new module Prevent Version Disclosure →
Thanks nod_ for the push in that direction. That way I don't have to keep re-rolling this patch for our use.
Crediting nod_ for the code gifted in ✨ Javascript Version Disclosure Active
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.