Skip to content

Conversation

@neilt1700
Copy link

No description provided.

EugenMayer and others added 3 commits March 21, 2023 12:02
- See d6lts#77 "Apache update
  causes Drupal URLs with spaces and other special characters to
  fail"
- (Removing warnings) - checking for existence of properties and
  array keys before attempting to use them (mysqli.inc, menu.inc,
  comment.module, node.module, user.module
- form.inc, update.php - only variables may be passed by reference
- install.php - order of arguments to implode must now be
  separator, array
@neilt1700 neilt1700 mentioned this pull request May 5, 2023
@amorsent
Copy link
Contributor

amorsent commented May 6, 2023

We found that simply adding the B flag to the .htaccess rewrite solved some issues, but caused others for us.

I can't entirely recall the specifics, but it had to do with filenames being double encoded by drupal_encode_path() I think.
If I recall, they were previously double encoded, but Apache would decode them.

Apache recently changed this behavior for security reasons (CVE-2023-25690).
Adding the B flag sort of fixes it, but causes issues for the stuff that was previously double encoded.

(I think I have that roughly accurate, but I'm a little fuzzy on the specifics)

We instead did a partial backport of the changes that went into D7 which entirely moves away from using the querystring at all.
https://git.drupalcode.org/project/drupal/-/commit/1df3cfffefefc93ed2d29041d148938d08bb9d4e

I do recall that that fix maintains $_GET['q'] for backward compatibility.
I also recall that we also set $_REQUEST['q'] because some contrib module was using that.

I will file a pull request with our patch when I get a chance...

@amorsent
Copy link
Contributor

amorsent commented May 6, 2023

BTW - This is the Drupal issue associated with the commit we backported:

https://www.drupal.org/project/drupal/issues/284899

@amorsent
Copy link
Contributor

amorsent commented May 7, 2023

Here is a pull request for the fix we did instead of the B flag.
#79

As I mentioned its a backport of what D7 does.
For us it solved the issues caused by the Apache security fix without causing new issues.

@neilt1700
Copy link
Author

Here is a pull request for the fix we did instead of the B flag. #79

As I mentioned its a backport of what D7 does. For us it solved the issues caused by the Apache security fix without causing new issues.

That's great. (I reverted the "B" flag change in the pull request - which otherwise still fixes some PHP8 compatibility problems).

@neilt1700 neilt1700 closed this May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants