Skip to content

Commit

Permalink
Allow uppercase characters in package slugs. Fixes #83
Browse files Browse the repository at this point in the history
  • Loading branch information
bradyvercher committed Nov 29, 2018
1 parent 950a500 commit be16e11
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/Route/Download.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function handle( Request $request ): Response {
throw HttpException::forForbiddenResource();
}

$slug = sanitize_key( $request['slug'] );
$slug = preg_replace( '/[^A-Za-z0-9_\-]+/i', '', $request['slug'] );
if ( empty( $slug ) ) {
throw HttpException::forUnknownPackage( $slug );
}
Expand Down

5 comments on commit be16e11

@afragen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe sanitize_file_name() will work here. It’s what I use to sanitize the key in GitHub Updater.

https://github.com/afragen/github-updater/blob/2d9595fb4e1b3e941f5fb1a5153d14ada74734de/src/GitHub_Updater/Traits/GHU_Trait.php#L280

@bradyvercher
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afragen Thanks for the suggestion! I'll have to look into that if anymore issues pop up related to unsupported characters.

@leepeterson
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started seeing notices such as this today:

Deprecation warning: require statements should not contain uppercase characters. 

Composer 2.0 will error out.

Any way to get around this in this in the short term?

@GaryJones
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradyvercher
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leepeterson This commit was to fix a bug unrelated to that Composer notice, which appears to be a new requirement as @GaryJones pointed out. I don't have any plugins installed that have uppercase characters in their slug, so I haven't run into this myself and I'm not exactly sure what it would take to lowercase slugs just yet, but I opened a new issue to track this.

In the meantime, you might be able to temporarily downgrade to Composer 1.8.0 if the notices are causing issues for you.

Can you let me know what plugin you're using that has uppercase characters in its directory name?

Please sign in to comment.