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.

Sign up for GitHub

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

Jump to bottom

Update XrdSciTokensAccess.cc #1859

Closed
wants to merge 5 commits into from
Closed

Update XrdSciTokensAccess.cc #1859

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
959573b
Update XrdSciTokensAccess.cc
biozit Dec 13, 2022
a2ee18c
Update XrdSciTokensAccess.cc
biozit Dec 13, 2022
4c5245b
Update XrdSciTokensAccess.cc
biozit Dec 13, 2022
280c9e7
Update XrdSciTokensAccess.cc
biozit Dec 13, 2022
5766bb6
Update XrdSciTokensAccess.cc
biozit Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/XrdSciTokens/XrdSciTokensAccess.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ class XrdAccSciTokens : public XrdAccAuthorize, public XrdSciTokensHelper
section.c_str());
continue;
}
// prevent two identical issuer break the config
// prevent two identical issuers break the config
for (size_t i = 0; i < issuer.length(); i++)
Copy link
Member

Choose a reason for hiding this comment

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

Why didn't you use index() and instead implemented a manual version of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about that?

Copy link
Member

Choose a reason for hiding this comment

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

Well, I was thinking of something much simpler. But before we get to that, I am wondering what is being fixed here. According to INIReader, the new line character ('\n') is considered a separator so there should never be a new line character in the value string. So, why is it that the code assumes otherwise?

Second, if indeed a new line can be in the value string then there is no easy solution here as the new line character can appear anywhere and multiple times as well. So, consider "\nissuer", "issuer\n", or "\nissuer\n" the fixes are very different in each case. The variations are endless (well actually two basic cases).

OK, simplicity (note we only handle the simple case here):
size_t i = issuer.find('\n');
if (i != std::string::npos) {
if (i != 0) issuer = issuer.substr(0, i);
else /error string starts with newline and we don't handle that */
}

Yes, I should have have mentioned index() -- old habits.

{
if (issuer[i] == '\n')
Expand Down