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.

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

XMLPullParserException on non XML error ResponseBody #718

Closed
NathanAtKadaster opened this issue Oct 25, 2018 · 7 comments · Fixed by #771
Closed

XMLPullParserException on non XML error ResponseBody #718

NathanAtKadaster opened this issue Oct 25, 2018 · 7 comments · Fixed by #771

Comments

@NathanAtKadaster
Copy link

NathanAtKadaster commented Oct 25, 2018

(This continues on #354 but I cannot re-open that issue so to not hijack that thread I made opened a new issue. )

So to clarify this a bit more. I stumbled upon this when trying to make a new bucket and the lack of logging is a bit of a pain since only after I found this thread (#354), I was able to find out what happened.

The isssue is that the client does not do any type of content negotiation and just tries to parse anything it gets back as XML.

So after enabling the client object to #traceOn System.out I found the following request and reponse:

PUT /*REDACTED* HTTP/1.1
Host: localhost:9000
User-Agent: Minio (amd64; amd64) minio-java/dev
x-amz-content-sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
x-amz-date: 20181025T094012Z
Authorization: AWS4-HMAC-SHA256 Credential=*REDACTED*/20181025/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=*REDACTED*

Response:

Cache-Control: max-age=31536000
Content-Type: text/plain; charset=utf-8
X-Content-Type-Options: nosniff
Date: Thu, 25 Oct 2018 09:40:13 GMT
Content-Length: 19

Now the exception is talking about this 4 thing which is most likely the 4 of 404. I find this really interesting. The response clearly states that it is just plain text. So why is the client trying to parse this as XML?

Digging in the code I found MinioClient#execute which always tries to parse the response body as XML. Except on a HEAD method (because no body).:

    // HEAD returns no body, and fails on parseXml
    if (!method.equals(Method.HEAD)) {
      Scanner scanner = new Scanner(response.body().charStream());
      try {
        scanner.useDelimiter("\\A");
        String errorXml = "";

        // read entire body stream to string.
        if (scanner.hasNext()) {
          errorXml = scanner.next();
        }

        errorResponse = new ErrorResponse(new StringReader(errorXml));
        if (this.traceStream != null) {
          this.traceStream.println(errorXml);
        }

      } finally {
        response.body().close();
        scanner.close();
      }
    }

Which could be rewritten to something like:

    // HEAD returns no body, and fails on parseXml
    if (!method.equals(Method.HEAD)) {
      try (String boy = response.body().string()) {
        if ("application/xml".equals(response.headers().get("content-type"))) {
          errorResponse = new ErrorResponse(new StringReader(body));
        }
        if (this.traceStream != null) {
          this.traceStream.println(body);
        }
      }
    }

This way parsing does not fail if the response is not XML.

@nitisht
Copy link
Contributor

nitisht commented Oct 25, 2018

@NathanAtKadaster do you mind sending a PR instead, that will be easier to review and possibly take in

@harshavardhana
Copy link
Member

So to clarify this a bit more. I stumbled upon this when trying to make a new bucket and the lack of logging is a bit of a pain since only after I found this thread (#354), I was able to find out what happened.

When you are creating the bucket what is the server sending the response here?

@NathanAtKadaster
Copy link
Author

@nitisht Yeah I think i'll be able to do so next week.

@harshavardhana Our own minio docker server. This can either be on the same machine (for unit-/integrationtesting we use TestContainers to spin up a minio server) or on a different host (where TestContainers handles the URL creation for a viable Host URL).

@kannappanr
Copy link
Contributor

@NathanAtKadaster Did you get a chance to work on the PR?

@deekoder
Copy link
Contributor

ping @NathanAtKadaster would you be able to re-look at this in 2019? We would love a pr from you for this issue.

@sinhaashish
Copy link
Contributor

@NathanAtKadaster According to S3 specification the server always sends an XML response and thus client expects XML response. In this case the server response is plain text. This seems to be a sever issue. I will check how the aws-java sdk behaves in this situation , but for that i need to replicate the issue first.
Please elaborate the steps performed which helps to replicate the issue.

@NathanAtKadaster
Copy link
Author

NathanAtKadaster commented Feb 5, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants