Problem/Motivation

This is a follow-up on #3416700: Handle invalid compressed ajax_page_state more gracefully for the following improvements:

  • Allow !== 0 /'s in the library name
  • Include information about the library that caused the BadRequestHttpException to make it easier to find the root cause as site administrator
  • Improve the exception message to be clearer about what's changed and wrong

See related issues, e.g. https://www.drupal.org/project/asset_injector/issues/3444733#comment-155...

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3445072

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Anybody created an issue. See original summary.

Anybody opened merge request !7901

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Implemented the suggestion from @longwave (from #3416700: Handle invalid compressed ajax_page_state more gracefully) in the MR and added some comments.
I can't go into details about the if clause and the message, as I don't really understand what it really does and what this can be caused by.

So I don't know what exactly we can tell in the Exception or not.

catch’s picture

catch
he/him
Primary language English
commented

A library has to be in the format foo/bar, so this is just detecting if someone puts somerandomstringthatisnotalibrary in the query parameter somewhere. We just overlooked that people apparently use foo/bar/baz sometimes, which is unexpected but possibly not wrong (or even if it's wrong worked until we broke it/

So something like Check that libraries are in the correct format should be fine I think?

longwave made their first commit to this issue’s fork.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review

Updated the comment and exception message to be more useful.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community

Thanks @longwave, RTBC from my side, this clearly points out what happens. Perhaps @catch should sign this off, then this is ready to go?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Slightly surprised PHPStan did not pick up the invalid use of formatting on commit, but oh well, the tests caught it.

  • catch committed fb257090 on 10.3.x 
    Issue #3445072 by longwave, Anybody: Improve "The libraries to include...

  • catch committed 186b5722 on 10.4.x 
    Issue #3445072 by longwave, Anybody: Improve "The libraries to include...

  • catch committed 803c9429 on 11.x 
    Issue #3445072 by longwave, Anybody: Improve "The libraries to include...
catch’s picture

catch
he/him
Primary language English
commented
Version: 10.3.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Much better. Committed/pushed to 11.x and cherry-picked to 10.4.x/10.3.x/10.2.x, thanks!

catch closed merge request !7901

  • catch committed 81671bc5 on 10.2.x 
    Issue #3445072 by longwave, Anybody: Improve "The libraries to include...

  • catch committed 803c9429 on 11.0.x 
    Issue #3445072 by longwave, Anybody: Improve "The libraries to include...
thejimbirch’s picture

thejimbirch commented

I ran into this on a 10.2.6 site. This was merged into 10.2.x a few days after that was released. So if you run into this until 10.2.7 comes out...

I patched drupal/core using a diff of the merge. The file URL for that is:

https://git.drupalcode.org/project/drupal/-/commit/81671bc5b9df29774a774...

Hope that helps.

The error message was:

Symfony\Component\HttpKernel\Exception\BadRequestHttpException: The libraries to include are encoded incorrectly. in Drupal\system\Controller\AssetControllerBase->Drupal\system\Controller\{closure}() (line 168 of /code/web/core/modules/system/src/Controller/AssetControllerBase.php).

We were getting 400 errors on the JS files causing Admin Toolbar to print without styles.

Thanks for the issue and the fix! This was a squirrely one!

Adding this patch fixed the problem. Never could identify which library had an issue, or if there was a real issue.

shawn dearmond’s picture

shawn dearmond commented
Related issues: +#3447501: Drupal 10.2.6 causes libraries to fail

We ran into this yesterday on a 10.2.6 site. In our case, it was the Decoupled Blocks module.

#3447501: Drupal 10.2.6 causes libraries to fail

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.