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

Require package names to be valid provides #1778

Merged
merged 1 commit into from Oct 14, 2021

Conversation

ffesti
Copy link
Contributor

@ffesti ffesti commented Sep 20, 2021

Only allow alphanumeric or _ as first character.
Also check the name of Obsoletes.

Resolves: #1694

build/parsePreamble.c Outdated Show resolved Hide resolved
build/rpmbuild_internal.h Outdated Show resolved Hide resolved
@@ -19,6 +19,7 @@
#undef HTDATATYPE

#define ALLOWED_CHARS_NAME ".-_+%{}"
#define ALLOWED_FIRSTCHARS_NAME "_%{}"
Copy link
Member

@pmatilai pmatilai Oct 13, 2021

Choose a reason for hiding this comment

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

Doh, I missed the actual beef of the change there (once again...)

While we need to accept %{} from macro leakage in the names, { and } are not actually valid as the first characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, after reading rpmCharCheck() 3 more times: Looks like %{}are actually legal in dependency names and only create a warning. So they must not be in the first character.

Copy link
Member

@pmatilai pmatilai Oct 14, 2021

Choose a reason for hiding this comment

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

Took a bit of digging, but here's the case for unexpanded macros in the names etc:
https://bugzilla.redhat.com/show_bug.cgi?id=547997 and commit 507f21f.

So as I tried to say in my previous comment, I think you'd still need to allow % as the first character.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so this is not by accident but to not blow up for unexpanded macros that may just not be available at that point in time.

Only allow  alphanumeric or _ as first character.
Also check the name of Obsoletes.

Resolves: rpm-software-management#1694
@pmatilai pmatilai merged commit 9cc5aad into rpm-software-management:master Oct 14, 2021
@pmatilai
Copy link
Member

Going forward, we should probably look into replacing this silly rpmCharCheck() thing with regular expressions...

@ffesti ffesti deleted the 1694 branch October 14, 2021 11:24
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.

rpmbuild should not accept package Name starting with '-'
3 participants