Skip to content

Conversation

@hlein
Copy link
Contributor

@hlein hlein commented Nov 10, 2021

This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger [email protected]
Bug: #3678
Bug: #3851
Bug: #4633

@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2021

This pull request introduces 1 alert when merging 1373136 into 9a81078 - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function

@hlein hlein force-pushed the firejail_envchecks branch from 1373136 to e4f1380 Compare November 10, 2021 22:28
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

Other than the suggestions, LGTM.

This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger <[email protected]>
Bug: netblue30#3678
Bug: netblue30#3851
Bug: netblue30#4633
@hlein hlein force-pushed the firejail_envchecks branch from e4f1380 to 0d06369 Compare November 10, 2021 22:58
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

LGTM.

@netblue30 netblue30 merged commit 98b8172 into netblue30:master Nov 11, 2021
@netblue30
Copy link
Owner

merged, thanks!

kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 21, 2023
Also, s/arguments/argument/ since the message refers to one specific
argument.

Relates to commit 0d06369 ("Make env/arg sanity check failure messages
more useful", 2021-11-10) / PR netblue30#4676.
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 21, 2023
Also, s/arguments/argument/ since the message refers to one specific
argument.

Relates to commit 0d06369 ("Make env/arg sanity check failure messages
more useful", 2021-11-10) / PR netblue30#4676.

Relates to netblue30#5676.
kmk3 added a commit to kmk3/firejail that referenced this pull request Nov 1, 2025
Replace the hardcoded `MAX_ENVS` and `MAX_ENV_LEN` limits with new
global configuration options, `env-max-count` and `env-max-len`, which
limit the maximum number of environment variables and the maximum length
of each environment variable (respectively).

Also, include the environment name and value in the "too long
environment variable" error message, similarly to the "too long
argument" error message (see PR netblue30#4676 and PR netblue30#5677).

This is a follow-up to netblue30#6878.

Closes netblue30#3678.
kmk3 added a commit that referenced this pull request Nov 1, 2025
Replace the hardcoded `MAX_ENVS` and `MAX_ENV_LEN` limits with new
global configuration options, `env-max-count` and `env-max-len`, which
limit the maximum number of environment variables and the maximum length
of each environment variable (respectively).

Also, include the environment name and value in the "too long
environment variable" error message, similarly to the "too long
argument" error message (see PR #4676 and PR #5677).

This is a follow-up to #6878.

Closes #3678.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants