#122 patch did not fix the issue for our page that's hosted on aws and we're getting 403 from the server. To make it harder to debug, the media library pagination works locally and on our CI.
I was thinking about writing an http_middleware that is checking the request size and creates a watchdog warning if request is to big. Then at least we'd know we're going to have problems on production environments.
This is how our request looks like:
GET /views/ajax?media_library_opener_id=media_library.opener.editor&media_library_allowed_types%5B0%5D=image&media_library_allowed_types%5Bdocument%5D=document&media_library_allowed_types%5Bgallery%5D=gallery&media_library_allowed_types%5Bvideo%5D=video&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfilter_format_id%5D=restricted_html&media_library_opener_parameters%5Btenant%5D=<id>&media_library_view=media_library&media_library_display=widget_table&hash=<hash>&_wrapper_format=drupal_modal&_wrapper_format=drupal_ajax&view_name=media_library&view_display_id=widget_table&view_args=image&view_path=%2Fmedia-library&view_base_path=admin%2Fcontent%2Fmedia-widget-table&view_dom_id=d9cf4f37403929685b2da3338f20b4af842d6a45f430e626c1acfaa79e4abc69&pager_element=0&media_library_opener_id=media_library.opener.editor&media_library_allowed_types%5B0%5D=image&media_library_allowed_types%5Bdocument%5D=document&media_library_allowed_types%5Bgallery%5D=gallery&media_library_allowed_types%5Bvideo%5D=video&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfilter_format_id%5D=restricted_html&media_library_opener_parameters%5Btenant%5D=<id>&media_library_view=media_library&media_library_display=widget_table&hash=<hash>&_wrapper_format=drupal_modal&page=1&_drupal_ajax=1&ajax_page_state%5Btheme%5D=<theme>&ajax_page_state%5Btheme_token%5D=<token>Q&ajax_page_state%5Blibraries%5D=<924chars-of-hash> HTTP/2
Host: cms.int.infoportal.cloud.wholesaleservices.de
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/115.0
Accept: application/json, text/javascript, */*; q=0.01
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
Referer: https://<domain>/node/add/article/<id>
X-Requested-With: XMLHttpRequest
Connection: keep-alive
Cookie: SSES<session-cookie> AWS<220-chars of was cookies>
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-origin
DNT: 1
Sec-GPC: 1
TE: trailers
and response is pretty simple:
HTTP/2 403 Forbidden
server: awselb/2.0
date: Thu, 20 Jun 2024 10:52:57 GMT
content-type: text/html
content-length: 118
X-Firefox-Spdy: h2
For me the patch # 27 🐛 Ajax Pager broken after upgrade 10.0.9 to 10.1.2 Needs work breaks some other views (because view_name parameter is not found).
But the underlying problem, why I'm looking into this issue is because aws is returning 403 on /views/ajax requests because url query is too long, so I'm looking into ways how to compress the query params length.
useernamee → created an issue.
merged.
will be part of release 2.0.2.
merged
creating a new release - 2.0.2
Taken care of in 🐛 Caching issue RTBC
During working on this ticket I noticed that drupal cache gets permanent status if expires header is not set, that's why added that one as well.
Added a quickfix.
useernamee → created an issue.
useernamee → created an issue.
We're using patch #11 and it fixes our issue.
We are using the patch from #4 and it fixes our issue. Thus moving it into RTBC.
Patch #6 fixes our issue.
Once tests are added I'd recommend moving this ticket into RTBC.
useernamee → created an issue.
Added to 2.0.1 release.
merged, fixed
Fixed php errors.
I'm seeing this error:
The website encountered an unexpected error. Try again later.
TypeError: Drupal\ohdear_integration\Controller\OhDearIntegrationController::refineCacheMetadata(): Argument #1 ($cacheable_dependency) must be of type Drupal\ohdear_integration\Controller\RefinableCacheableDependencyInterface, Drupal\Core\Access\AccessResultAllowed given, called in /app/web/modules/contrib/ohdear_integration/src/Controller/OhDearIntegrationController.php on line 202 in Drupal\ohdear_integration\Controller\OhDearIntegrationController->refineCacheMetadata() (line 212 of modules/contrib/ohdear_integration/src/Controller/OhDearIntegrationController.php).
Drupal\ohdear_integration\Controller\OhDearIntegrationController->access(Object) (Line: 108)
Drupal\ohdear_integration\Controller\OhDearIntegrationController->Drupal\ohdear_integration\Controller\{closure}() (Line: 627)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 136)
Drupal\ohdear_integration\Controller\OhDearIntegrationController->buildJson()
If you have http_cache_control module installed, you need this patch: 🐛 Allow s-maxage to be overridden RTBC
I found this patch and it is exactly what we were looking for. RTBC.
Implemented the change from ✨ Add support for Lupus Decoupled Drupal Needs review
Even better!
I've created a MR on 📌 Improve debugging/developer experience with rest_log module Needs review and tested the issue locally. It worked well. I did also create a MR.
I did not really need the currentRouteMatch in check in our log check (https://git.drupalcode.org/project/lupus_decoupled/-/merge_requests/83/d...) but I don't really see it as an issue.
I added the rest log in require-dev and suggest in composer.json.
For checking api requests I've added a request check:
protected function isApiResponse(Request $request): bool {
return $request->attributes->get('lupus_ce_renderer') || $request->getRequestFormat() == 'custom_elements';
}
I've also been considering some other ways, but this seemed the most straightforward. The module now works just on on/off - if enabled it logs and if not it doesn't. I think that for development experience this is enough.
Remarks are fixed.
needs review
useernamee → created an issue.
Thank you @ad0z for improvements, I checked PR and it looks good to me. I've created a Lupus Decoupled Drupal issue to implement it 📌 Improve debugging/developer experience with rest_log module Needs review . I'll move this ticket into RTBC after I test it with Lupus Decoupled Drupal.
useernamee → created an issue.
There is also an issue when visiting /admin/reports/ohdear/info. The site breaks if api key is wrong or missing. Controller probably needs some error handling or the ohdear service.
Unfortunately the patch does not work for me. If I try to edit the fields of specific content/bundle, after saving the form all the fields are hidden and if I check and uncheck the bundle all the fields checked once again.
Anyways I think that the data is correct even though once the form reloads you can't see which fields are (un)checked. I did de-select some paragraphs fields, saved the form and then checked the values that were visually hidden and the fields were (un)checked correctly as I set them.
I guess it is inconvenient not to be able to see the field settings but the configuration is saved correctly.
I propose to add voter design pattern to determine whether to log an request or not.
merged
It was tested by Martin.
useernamee → created an issue.
Code changes looks good. @martin is there some test envrionment to test it?
I just don't understand why you remove elements from get parameters: https://git.drupalcode.org/project/migrate_log_ui/-/merge_requests/3/dif...
merged.
merged & will be a part of next release
Code looks good. Works on our project.
Merged, will be a part of next release.
Looks good, works on our system.
merged, will be part of next release
We are using this patch on our Drupal 10 project. RTBC.
@jamesgrobertson changes look good to me, except for the missing changes in the DeleteLogGroupForm which I plan to add manually.
@aludescher would you agree that we add this small change to your patch:
diff -u b/src/Logger/Log.php b/src/Logger/Log.php
--- b/src/Logger/Log.php
+++ b/src/Logger/Log.php
@@ -179,7 +183,16 @@
$this->putLog->putLog(
$aws_client,
- $message_formatted,
+ $event->getMessage(),
$event->getLogGroup(),
$event->getLogStream()
);
This code is already running on system so I'm marking this ticket RTBC.
Patch applied to 8.x-1.x main branch.
Changes are good and we're using this patch on our system. -> RTBC
Merged
I guess the way forward would be to make RestLogSubscriber::isRestPage method easily extensible from outside with SOLID principles and rename it to sth like applies.
useernamee → created an issue.
useernamee → created an issue.
useernamee → created an issue.
Reviewed and it looks good.
Thank you @casey & @roderik for you contributions.
Had a minor issue in the patch and I fixed that.
I've re-rolled the patch for 10.2.3 version
Well, I wouldn't mind if there'd be some kind of log message that would log if a library name is malformed such as it was the case in 🐛 Fix not loading bootstrap_styles/aos.local or bootstrap_styles/aos.remote library Fixed . It would be easier to debug such occurrences.
I've also created a patch so that it can be added to project composer patches.
I've investigated the issue a bit and found the max batch size limit on PutLogEvents.
https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API...
So I've added some logic that truncates the message to 256kb if it is too long.
useernamee → created an issue.
merged
I've added the proposed changes.
Patch fixes issue on our CI environment.
useernamee → created an issue.
useernamee → created an issue.
I've checked the patch.
ohdear_integration.settings service should be removed as well from services.yml file.
Then from:
- https://git.drupalcode.org/project/ohdear_integration/-/blob/2.x/src/OhD...
Otherwise the patch looks good. It just need a few more things to take care of.
Merged & fixed.
PR looks good and it will fix phpstan errors. RTBC.
composer phpstan-check web/modules/contrib/lupus_ce_renderer
> vendor/bin/phpstan analyze --memory-limit=-1 'web/modules/contrib/lupus_ce_renderer'
Note: Using configuration file /home/len/Projects/drunomix/ldp/ldp-project/phpstan.neon.
19/19 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
------ -----------------------------------------------------------------------------------------------------------
Line src/CustomElementsMetatagsGenerator.php
------ -----------------------------------------------------------------------------------------------------------
56 Access to an undefined property Drupal\lupus_ce_renderer\CustomElementsMetatagsGenerator::$moduleHandler.
💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
101 Access to an undefined property Drupal\lupus_ce_renderer\CustomElementsMetatagsGenerator::$moduleHandler.
💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
------ -----------------------------------------------------------------------------------------------------------
useernamee → created an issue.
All points are merged/done in this ticket, fixed.
@casey - good point - should we add some checkbox to tell the module that the site is not online? Or maybe we should just handle missing API key differently (e.g. we could move the functionality into submodule).
Are there any good examples from other modules, how they handle similar scenarios?
merged
I've created the new patch.
I've implemented the last comment.
Merged
Alternatively we could use:
if (($entity instanceof ContentEntityInterface || $entity instanceof WebformInterface) &&
$entity->hasLinkTemplate('canonical') &&
// Only add alternate link if entity is translated.
count($entity->getTranslationLanguages()) > 1) {
but I don't like this solution either. getTranslationLanguages is defined on WebformInterface and the problem with this is that it is too specific for my taste. It might cause issues if webform is not installed.
ContentEntityInterface (ultimately) inherits the method from web/core/lib/Drupal/Core/TypedData/TranslatableInterface.php but that is not the case for Config entities.
This solution works for me, but I've just figured it out that it only adds webform translations if content_translation module is installed which is not optimal. https://git.drupalcode.org/project/lupus_ce_renderer/-/merge_requests/33...
This condition might need some more consideration.
I've quickly checked: https://git.drupalcode.org/project/metatag/-/blob/2.0.x/metatag.module?r... metatag_generate_entity_all_tags should be used because it also provides the alter hook and the manager does not.
Plz just drop the metatagManager service since it is not used in the class.
@junkuncz thank you for you PR.
I've quickly checked changed files and I've noticed that in the CustomElementsMetatagsGenerator there is MetatagManager service injected but never used. Either this should go out we use it instead of the deprecated metatag_generate_entity_metatags function.
Summary looks good to me and I've added it to the module description. I've also added a logo to the codebase.
useernamee → made their first commit to this issue’s fork.