Print

Print


@abh3 commented on this pull request.

So, now I am a bit confused as what we are trying to fix. More comments inside.

> @@ -1083,6 +1083,16 @@ class XrdAccSciTokens : public XrdAccAuthorize, public XrdSciTokensHelper
                      section.c_str());
                 continue;
             }
+	    // prevent two identical issuers break the config 
+	    for (size_t i = 0; i < issuer.length(); i++)

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.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/xrootd/xrootd/pull/1859#pullrequestreview-1214751508
You are receiving this because you are subscribed to this thread.

Message ID: <[log in to unmask]>

########################################################################
Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1