Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter allowed process type characters #237

Merged
merged 2 commits into from
Mar 19, 2025

Conversation

runesoerensen
Copy link
Contributor

@runesoerensen runesoerensen commented Mar 19, 2025

We currently use the project assembly name to build launch process types. However, assembly names can include several characters that are not allowed in the CNB spec.

This PR changes the launch process type to only include characters allowed by the CNB spec, eliminating the need to warn users when an invalid character is included in the assembly name. It also allows us to safely make the detect_solution_process infallible, reducing code complexity in the process.

Future improvements to consider:

  • While somewhat unlikely, this change could cause errors if the sanitized launch process names for two or more projects are identical (e.g. foo+bar.csproj, foobar.csproj, foo bar.csproj will all use the foobar process type name), which the CNB spec doesn't allow.
  • As noted in Handle process types names containing . and upper-case characters #188, we may want to also filter out ., even if it's allowed under the CNB spec, to increase compatibility with the Procfile spec (which only allows alphanumeric characters + hyphens and underscores). Using . in .NET project names (e.g. MyApp.Frontend) is fairly common, so this is more likely to cause issues for users of this buildpack.
  • On that note, we may also want to always use lower case process types (as process types with upper cased characters has historically caused issues on certain platforms).

@runesoerensen runesoerensen changed the title Sanitize derived launch process type names Filter allowed process type characters Mar 19, 2025
@runesoerensen runesoerensen force-pushed the sanitize-launch-process-type branch from 327c0cd to e565d36 Compare March 19, 2025 06:46
@runesoerensen runesoerensen force-pushed the refactor-launch-process-detection branch from d5f67b2 to 8ba461d Compare March 19, 2025 06:47
@runesoerensen runesoerensen force-pushed the sanitize-launch-process-type branch from e565d36 to cb4eb55 Compare March 19, 2025 06:47
Base automatically changed from refactor-launch-process-detection to main March 19, 2025 09:04
@runesoerensen runesoerensen force-pushed the sanitize-launch-process-type branch from cb4eb55 to 04c1da9 Compare March 19, 2025 09:07
@runesoerensen runesoerensen marked this pull request as ready for review March 19, 2025 09:08
@runesoerensen runesoerensen requested a review from a team as a code owner March 19, 2025 09:08
Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

Future improvements to consider:

Longer term we'll also likely need to update a few things (here and in other components) due to the upcoming RFC1123 subdomain format validation change:
https://github.com/heroku/lyra-runtime/pulls?q=is%3Apr+RFC1123
https://salesforce-internal.slack.com/archives/CCKDG0FAT/p1741792195879209
https://salesforce-internal.slack.com/archives/CAJNZ67FF/p1733907948659819

@runesoerensen runesoerensen merged commit 587e07f into main Mar 19, 2025
6 checks passed
@runesoerensen runesoerensen deleted the sanitize-launch-process-type branch March 19, 2025 09:25
heroku-linguist bot added a commit that referenced this pull request Mar 20, 2025
## heroku/dotnet

### Changed

- The buildpack now sanitizes launch process type names, based on project assembly names, by filtering out invalid characters. ([#237](#237))
- Launch process commands with paths containing special characters (including spaces) are now properly quoted. ([#239](#239))
- The `test` launch process, added when targeting the test execution environment, now properly handles solution/project filenames containing special characters (including spaces). ([#240](#240))
@heroku-linguist heroku-linguist bot mentioned this pull request Mar 20, 2025
heroku-linguist bot added a commit that referenced this pull request Mar 20, 2025
## heroku/dotnet

### Changed

- The buildpack now sanitizes launch process type names, based on project assembly names, by filtering out invalid characters. ([#237](#237))
- Launch process commands with paths containing special characters (including spaces) are now properly quoted. ([#239](#239))
- The `test` launch process, added when targeting the test execution environment, now properly handles solution/project filenames containing special characters (including spaces). ([#240](#240))

Co-authored-by: heroku-linguist[bot] <136119646+heroku-linguist[bot]@users.noreply.github.com>
heroku-linguist bot added a commit to heroku/cnb-builder-images that referenced this pull request Mar 20, 2025
## heroku/dotnet

### Changed

- The buildpack now sanitizes launch process type names, based on project assembly names, by filtering out invalid characters. ([#237](heroku/buildpacks-dotnet#237))
- Launch process commands with paths containing special characters (including spaces) are now properly quoted. ([#239](heroku/buildpacks-dotnet#239))
- The `test` launch process, added when targeting the test execution environment, now properly handles solution/project filenames containing special characters (including spaces). ([#240](heroku/buildpacks-dotnet#240))
heroku-linguist bot added a commit to heroku/cnb-builder-images that referenced this pull request Mar 20, 2025
## heroku/dotnet

### Changed

- The buildpack now sanitizes launch process type names, based on project assembly names, by filtering out invalid characters. ([#237](heroku/buildpacks-dotnet#237))
- Launch process commands with paths containing special characters (including spaces) are now properly quoted. ([#239](heroku/buildpacks-dotnet#239))
- The `test` launch process, added when targeting the test execution environment, now properly handles solution/project filenames containing special characters (including spaces). ([#240](heroku/buildpacks-dotnet#240))

Co-authored-by: heroku-linguist[bot] <136119646+heroku-linguist[bot]@users.noreply.github.com>
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.

2 participants