403 fix #44
403 fix #44
Conversation
…ut is incomplete.
…ut is incomplete.
…ut is incomplete.
…ut is incomplete.
…ut is incomplete.
# Conflicts: # src/S3File.cc
There was a problem hiding this comment.
Some cleanup of the approach is needed, though the fix looks correct.
Before pushing revisions, please tidy up the branch following the OSG advice on git branch maintenance:
Particularly, please pay attention to commit messages and tidying up in-progress work.
|
|
||
| AmazonRequest(const std::string &s, const std::string &akf, | ||
| const std::string &skf, const std::string &b, | ||
| const std::string &o, const std::string &style, int sv, | ||
| XrdSysError &log) | ||
| XrdSysError &log, bool ro = true) |
There was a problem hiding this comment.
This constructor change doesn't appear to be used. Let's scrap it.
There was a problem hiding this comment.
This constructor is used by the courtesy constructor that has a simpler interface, does some translations and then calls this one. This is actually where the bookkeeping is done for the retention policy.
src/S3Commands.hh
Outdated
| @@ -260,7 +263,9 @@ class AmazonS3List : public AmazonRequest { | |||
| public: | |||
| AmazonS3List(const S3AccessInfo &ai, const std::string &objectName, | |||
| size_t maxKeys, XrdSysError &log) | |||
| : AmazonRequest(ai, objectName, log), m_maxKeys(maxKeys) {} | |||
| : AmazonRequest(ai, objectName, log, false), m_maxKeys(maxKeys) { | |||
| retain_object = false; | |||
There was a problem hiding this comment.
This sets retainObject in two different ways. Pick one.
If you keep the protected variable, do something like this instead:
: AmazonRequest(ai, objectName, log, false), m_maxKeys(maxKeys), retainObject(false) {
src/HTTPCommands.cc
Outdated
| @@ -380,6 +427,8 @@ bool HTTPRequest::sendPreparedRequest(const std::string &protocol, | |||
| } | |||
| return false; | |||
| } | |||
| // rv = curl_easy_setopt(curl.get(), CURLOPT_DEBUGFUNCTION, debug_callback); | |||
There was a problem hiding this comment.
- Pass the logging object to the debug function and use the logging facility instead.
- Hook the request logging into the configuration subsystem (take a look into https://github.com/PelicanPlatform/xrootd-s3-http/blob/main/src/logging.cc; perhaps add a new
dumportracelevel?).
There was a problem hiding this comment.
I understand the second bullet and can make that happen. Also, yes, it almost has to be a new level because this will result in crazy large logfiles and probably a bit of latency.
I don't think I get the first bullet though. We are using a libcurl call here to register the debug callback. How would we pass a logging object into that library and have it be available when the callbacks are made?
Co-authored-by: Brian P Bockelman <[email protected]>
…are produced. added another logging mask.
# Conflicts: # src/S3Commands.hh
Repairs a 403 error listing files from S3. Also adds some debug code that proved very useful in finding the bug. The debug code is currently commented out.