fix: improve OSS Index Error Reporting#7977
Conversation
resolves #7976
|
For the issues #7976 shouldn't be managed in the code also the status code 407 ? (Proxy Authorization Required error) |
|
The original code had a few paths that would end up throwing the exception without checking the warn only flag. The updated version doesn't - even though we aren't specifically checking for a 407 it won't hit a path that throws the exception if you configure warn only. |
|
Thanks @jeremylong for keeping the tool working! |
core/src/main/java/org/owasp/dependencycheck/analyzer/OssIndexAnalyzer.java
Outdated
Show resolved
Hide resolved
| requestDelay(); | ||
| reports = requestReports(engine.getDependencies()); | ||
| } catch (TransportException ex) { | ||
| } catch (SocketTimeoutException e) { |
There was a problem hiding this comment.
I understand that the previous code also only specifically looked at SocketTimeoutException but it occurs to me that there are probably a number of other IOExceptions which essentially relate to an issue with the remote. I guess they will be handled by the generic catch (Exception ex) block, and depends how many the client wraps with TransportException which is a bit messy to unpack. Still this is not 'worse ' :-)
There was a problem hiding this comment.
The code was checking for a 401 inside the TransportExceptions - but it was not a transport exception so the code never worked. I agree, this isn't "worse" but we could do better.
Description of Change
ODC was not properly reporting authorization error messages. Previously, it was looking for the message to end with 401 to report an authorization issue. However, the actual error message from the client is
Failed to request component-reports: https://ossindex.sonatype.org/api/v3/component-report - Server status: 401 - Server reason: Unauthorized. As such, the check for 401 needed to be improved.Related issues
Best guess this is related to #7975 and possibly #7971
Have test cases been added to cover the new functionality?
Yes