oras-project / rust-oci-client

A Rust crate to interact with OCI registries
Apache License 2.0
86 stars 45 forks source link

validate_registry_response function #132

Open IIEIIEJI opened 2 months ago

IIEIIEJI commented 2 months ago

Hello!. Maybe, in the _validate_registryresponse function, there is a non-compliance with the https://github.com/opencontainers/distribution-spec/blob/main/spec.md#errors-2 standard. Firstly the returned format may not be json ( ...A 4XX response code from the registry MAY return a body in any format....). Secondly some registries return their own code, for example Harbor v2.9.0 returns this error: r#"{"errors":[{"code": "NOT_FOUND", "message": "artifact image:0.0.1 not found"}]}]}"#. It may be more correct to use the following variant of the _validate_registryresponse function:

fn validate_registry_response(status: reqwest::StatusCode, body: &[u8], url: &str) -> Result<()> {
    debug!(STATUS=?status);
    match status {
        reqwest::StatusCode::OK => Ok(()),
        reqwest::StatusCode::UNAUTHORIZED => Err(OciDistributionError::UnauthorizedError {
            url: url.to_string(),
        }),
        s if s.is_success() => Err(OciDistributionError::SpecViolationError(format!(
            "Expected HTTP Status {}, got {} instead",
            reqwest::StatusCode::OK,
            status,
        ))),
        s if s.is_client_error() => {
            let text = std::str::from_utf8(body)?;

            // According to the OCI spec, we should see an error in the message body.
            match serde_json::from_str::<OciEnvelope>(text) {
                Ok(envelope) => Err(OciDistributionError::RegistryError {
                    envelope,
                    url: url.to_string(),
                }),
                Err(err) => {
                    debug!(err=?err,"Not valid json format of RegistryError (https://github.com/opencontainers/distribution-spec/blob/master/spec.md#errors-2)");
                    Err(OciDistributionError::ServerError {
                        code: s.as_u16(),
                        url: url.to_string(),
                        message: text.to_string(),
                    })
                }
            }
        }
        s => {
            let text = std::str::from_utf8(body)?;

            Err(OciDistributionError::ServerError {
                code: s.as_u16(),
                url: url.to_string(),
                message: text.to_string(),
            })
        }
    }
}
thomastaylor312 commented 1 month ago

Looks like you are correct here! Code also looks pretty good with the only suggestion being to check the response headers to see if content-type is set before parsing JSON. Completely open to a PR if you'd like to submit!