Skip to content

Commit

Permalink
fix: update S3 status 200 error behavior (#4654)
Browse files Browse the repository at this point in the history
* fix: update S3 status 200 error behavior

* fix: exit is200Error early if response too short/long
  • Loading branch information
kuhe authored Jul 11, 2024
1 parent e4e0c12 commit 3344709
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-s3-09efbe2a.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "bugfix",
"category": "s3",
"description": "update s3 status 200 error classification"
}
57 changes: 51 additions & 6 deletions lib/services/s3.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,17 +531,21 @@ AWS.util.update(AWS.S3.prototype, {
* @api private
*/
extractErrorFrom200Response: function extractErrorFrom200Response(resp) {
if (!operationsWith200StatusCodeError[resp.request.operation]) return;
var service = this.service ? this.service : this;
if (!service.is200Error(resp) && !operationsWith200StatusCodeError[resp.request.operation]) {
return;
}
var httpResponse = resp.httpResponse;
if (httpResponse.body && httpResponse.body.toString().match('<Error>')) {
var bodyString = httpResponse.body && httpResponse.body.toString() || '';
if (bodyString && bodyString.indexOf('</Error>') === bodyString.length - 8) {
// Response body with '<Error>...</Error>' indicates an exception.
// Get S3 client object. In ManagedUpload, this.service refers to
// S3 client object.
resp.data = null;
var service = this.service ? this.service : this;
service.extractError(resp);
resp.error.is200Error = true;
throw resp.error;
} else if (!httpResponse.body || !httpResponse.body.toString().match(/<[\w_]/)) {
} else if (!httpResponse.body || !bodyString.match(/<[\w_]/)) {
// When body is empty or incomplete, S3 might stop the request on detecting client
// side aborting the request.
resp.data = null;
Expand All @@ -552,13 +556,54 @@ AWS.util.update(AWS.S3.prototype, {
}
},

/**
* @api private
* @param resp - to evaluate.
* @return true if the response has status code 200 but is an error.
*/
is200Error: function is200Error(resp) {
var code = resp && resp.httpResponse && resp.httpResponse.statusCode;
if (code !== 200) {
return false;
}
try {
var req = resp.request;
var outputMembers = req.service.api.operations[req.operation].output.members;
var keys = Object.keys(outputMembers);
for (var i = 0; i < keys.length; ++i) {
var member = outputMembers[keys[i]];
if (member.type === 'binary' && member.isStreaming) {
return false;
}
}

var body = resp.httpResponse.body;
if (body && body.byteLength !== undefined) {
if (body.byteLength < 15 || body.byteLength > 3000) {
// body is too short or long to be an error message.
return false;
}
}
if (!body) {
return false;
}
var bodyString = body.toString();
if (bodyString.indexOf('</Error>') === bodyString.length - 8) {
return true;
}
} catch (e) {
return false;
}
return false;
},

/**
* @return [Boolean] whether the error can be retried
* @api private
*/
retryableError: function retryableError(error, request) {
if (operationsWith200StatusCodeError[request.operation] &&
error.statusCode === 200) {
if (error.is200Error ||
(operationsWith200StatusCodeError[request.operation] && error.statusCode === 200)) {
return true;
} else if (request._requestRegionForBucket &&
request.service.bucketRegionCache[request._requestRegionForBucket]) {
Expand Down
14 changes: 7 additions & 7 deletions scripts/region-checker/allowlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ var allowlist = {
263,
276,
282,
642,
644,
763,
774,
775,
776,
781
687,
689,
808,
819,
820,
821,
826
],
'/token/sso_token_provider.js': [
60
Expand Down

0 comments on commit 3344709

Please sign in to comment.