Skip to content

Use RFC 822 format for marshalling date shapes #3452

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 4, 2022
Merged

Use RFC 822 format for marshalling date shapes #3452

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69e21d2
Use RFC 822 format for marshalling date shapes
dave-fn Sep 30, 2022
2fc5e5c
Explicilty set ResolverStyle and Chronology
dave-fn Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-83d35fd.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "dave-fn",
"type": "bugfix",
"description": "Build headers with two-digit day of month to meet RFC 822 reporting requirement"
}
4 changes: 2 additions & 2 deletions ...-core/src/test/java/software/amazon/awssdk/awscore/exception/AwsServiceExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void assertSkewed(int clientSideTimeOffset,
int statusCode,
Instant serverDate) {
AwsServiceException exception = exception(clientSideTimeOffset, errorCode, statusCode,
DateUtils.formatRfc1123Date(serverDate));
DateUtils.formatRfc822Date(serverDate));
assertThat(exception.isClockSkewException()).isTrue();
}

Expand All @@ -96,7 +96,7 @@ public void assertNotSkewed(int clientSideTimeOffset,
int statusCode,
Instant serverDate) {
AwsServiceException exception = exception(clientSideTimeOffset, errorCode, statusCode,
DateUtils.formatRfc1123Date(serverDate));
DateUtils.formatRfc822Date(serverDate));
assertThat(exception.isClockSkewException()).isFalse();
}

Expand Down
2 changes: 1 addition & 1 deletion core/aws-core/src/test/java/software/amazon/awssdk/awscore/retry/AwsRetryPolicyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private Consumer<RetryPolicyContext.Builder> applyErrorCode(String errorCode) {

private Consumer<RetryPolicyContext.Builder> applyErrorCode(String errorCode, Duration clockSkew, Instant dateHeader) {
SdkHttpFullResponse response = SdkHttpFullResponse.builder()
.putHeader("Date", DateUtils.formatRfc1123Date(dateHeader))
.putHeader("Date", DateUtils.formatRfc822Date(dateHeader))
.build();

AwsErrorDetails errorDetails = AwsErrorDetails.builder()
Expand Down
2 changes: 1 addition & 1 deletion ...ava/software/amazon/awssdk/protocols/json/internal/marshall/SimpleTypeJsonMarshaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void marshall(Boolean val, StructuredJsonGenerator jsonGenerator, JsonMar
jsonGenerator.writeNumber(DateUtils.formatUnixTimestampInstant(val));
break;
case RFC_822:
jsonGenerator.writeValue(DateUtils.formatRfc1123Date(val));
jsonGenerator.writeValue(DateUtils.formatRfc822Date(val));
break;
case ISO_8601:
jsonGenerator.writeValue(DateUtils.formatIso8601Date(val));
Expand Down
2 changes: 1 addition & 1 deletion ...ls/protocol-core/src/main/java/software/amazon/awssdk/protocols/core/InstantToString.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public String convert(Instant val, SdkField<Instant> sdkField) {
case ISO_8601:
return DateUtils.formatIso8601Date(val);
case RFC_822:
return DateUtils.formatRfc1123Date(val);
return DateUtils.formatRfc822Date(val);
case UNIX_TIMESTAMP:
return DateUtils.formatUnixTimestampInstant(val);
default:
Expand Down
2 changes: 1 addition & 1 deletion ...ls/protocol-core/src/main/java/software/amazon/awssdk/protocols/core/StringToInstant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public Instant convert(String value, SdkField<Instant> field) {
case UNIX_TIMESTAMP_MILLIS:
return safeParseDate(DateUtils::parseUnixTimestampMillisInstant).apply(value);
case RFC_822:
return DateUtils.parseRfc1123Date(value);
return DateUtils.parseRfc822Date(value);
default:
throw SdkClientException.create("Unrecognized timestamp format - " + format);
}
Expand Down
2 changes: 1 addition & 1 deletion core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/ClockSkew.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public static Optional<Instant> getServerTime(SdkHttpResponse serviceResponse) {
log.debug(() -> "Reported service date: " + serverDate);

try {
return Optional.of(DateUtils.parseRfc1123Date(serverDate));
return Optional.of(DateUtils.parseRfc822Date(serverDate));
} catch (RuntimeException e) {
log.warn(() -> "Unable to parse clock skew offset from response: " + serverDate, e);
return Optional.empty();
Expand Down
2 changes: 1 addition & 1 deletion ...-core/src/test/java/software/amazon/awssdk/core/internal/retry/ClockSkewAdjusterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void badDateTranslatesToZero() {

private SdkHttpFullResponse responseWithDateOffset(int value, ChronoUnit unit) {
return SdkHttpFullResponse.builder()
.putHeader("Date", DateUtils.formatRfc1123Date(Instant.now().plus(value, unit)))
.putHeader("Date", DateUtils.formatRfc822Date(Instant.now().plus(value, unit)))
.build();
}

Expand Down
4 changes: 2 additions & 2 deletions ...rc/test/java/software/amazon/awssdk/protocol/tests/clockskew/ClockSkewAdjustmentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private void stubForResponse(Instant serviceTime, int statusCode, String errorCo
.willReturn(aResponse()
.withStatus(statusCode)
.withHeader("x-amzn-ErrorType", errorCode)
.withHeader("Date", DateUtils.formatRfc1123Date(serviceTime))
.withHeader("Date", DateUtils.formatRfc822Date(serviceTime))
.withBody("{}")));
}

Expand All @@ -180,7 +180,7 @@ private void stubForClockSkewFailureThenSuccess(Instant serviceTime, int statusC
.willReturn(aResponse()
.withStatus(statusCode)
.withHeader("x-amzn-ErrorType", errorCode)
.withHeader("Date", DateUtils.formatRfc1123Date(serviceTime))
.withHeader("Date", DateUtils.formatRfc822Date(serviceTime))
.withBody("{}")));

stubFor(post(urlEqualTo(PATH))
Expand Down
40 changes: 40 additions & 0 deletions utils/src/main/java/software/amazon/awssdk/utils/DateUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.time.format.DateTimeParseException;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.annotations.ThreadSafe;

Expand All @@ -48,6 +49,18 @@ public final class DateUtils {
.toFormatter()
.withZone(UTC);

/**
* RFC 822 date/time formatter.
*/
static final DateTimeFormatter RFC_822_DATE_TIME = new DateTimeFormatterBuilder()
.parseCaseInsensitive()
.parseLenient()
.appendPattern("EEE, dd MMM yyyy HH:mm:ss")
.appendLiteral(' ')
.appendOffset("+HHMM", "GMT")
.toFormatter()
.withLocale(Locale.US);
Copy link
Contributor

@joviegas joviegas Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I guess the reason for Locale.US is V1-refernce, correct me if I am wrong.
  • Why are we not doing
toFormatter(ResolverStyle.SMART, IsoChronology.INSTANCE)

same as done in RFC_1123_DATE_TIME ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Right, Locale.US was chosen to match V1.
  • Unfortunately, that access modifier on that method is package access. The two (1, 2) public toFormatter methods do default to ResolverStyle.SMART as of Java 8, though.

Copy link
Contributor Author

@dave-fn dave-fn Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess an alternative would be to set the ResolverStye.SMART and IsoChonology.INSTANCE values on the DateTimeFormatter object after it's been built. Let me know if you'd prefer me to make that change.

Copy link
Contributor

@joviegas joviegas Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap , we can set it after its been built.
Can we add some test cases where we can test this too (ResolverStye and IsoChonology)?
We can compare the same with RFC_1123_DATE_TIME


// ISO_INSTANT does not handle offsets in Java 12-. See https://bugs.openjdk.java.net/browse/JDK-8166138
private static final List<DateTimeFormatter> ALTERNATE_ISO_8601_FORMATTERS =
Arrays.asList(ISO_INSTANT, ALTERNATE_ISO_8601_DATE_FORMAT, ISO_OFFSET_DATE_TIME);
Expand Down Expand Up @@ -102,6 +115,33 @@ public static String formatIso8601Date(Instant date) {
return ISO_INSTANT.format(date);
}

/**
* Parses the specified date string as an RFC 822 date and returns the Date object.
*
* @param dateString
* The date string to parse.
*
* @return The parsed Date object.
*/
public static Instant parseRfc822Date(String dateString) {
if (dateString == null) {
return null;
}
return parseInstant(dateString, RFC_822_DATE_TIME);
}

/**
* Formats the specified date as an RFC 822 string.
*
* @param instant
* The instant to format.
*
* @return The RFC 822 string representing the specified date.
*/
public static String formatRfc822Date(Instant instant) {
return RFC_822_DATE_TIME.format(ZonedDateTime.ofInstant(instant, UTC));
}

/**
* Parses the specified date string as an RFC 1123 date and returns the Date
* object.
Expand Down
41 changes: 40 additions & 1 deletion utils/src/test/java/software/amazon/awssdk/utils/DateUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static java.time.ZoneOffset.UTC;
import static java.time.format.DateTimeFormatter.ISO_INSTANT;
import static java.time.format.DateTimeFormatter.ISO_OFFSET_DATE_TIME;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static software.amazon.awssdk.utils.DateUtils.ALTERNATE_ISO_8601_DATE_FORMAT;
Expand Down Expand Up @@ -72,6 +73,44 @@ public void formatIso8601Date() throws ParseException {
assertEquals(expectedDate, actualDate);
}

@Test
public void formatRfc822Date_DateWithTwoDigitDayOfMonth_ReturnsFormattedString() throws ParseException {
String string = DateUtils.formatRfc822Date(INSTANT);
Instant parsedDateAsInstant = LONG_DATE_FORMAT.parse(string).toInstant();
assertThat(parsedDateAsInstant).isEqualTo(INSTANT);
}

@Test
public void formatRfc822Date_DateWithSingleDigitDayOfMonth_ReturnsFormattedString() throws ParseException {
Instant INSTANT_SINGLE_DIGIT_DAY_OF_MONTH = Instant.ofEpochMilli(1399484606000L);;
String string = DateUtils.formatRfc822Date(INSTANT_SINGLE_DIGIT_DAY_OF_MONTH);
Instant parsedDateAsInstant = LONG_DATE_FORMAT.parse(string).toInstant();
assertThat(parsedDateAsInstant).isEqualTo(INSTANT_SINGLE_DIGIT_DAY_OF_MONTH);
}

@Test
public void formatRfc822Date_DateWithSingleDigitDayOfMonth_ReturnsStringWithZeroLeadingDayOfMonth() throws ParseException {
final Instant INSTANT_SINGLE_DIGIT_DAY_OF_MONTH = Instant.ofEpochMilli(1399484606000L);;
String string = DateUtils.formatRfc822Date(INSTANT_SINGLE_DIGIT_DAY_OF_MONTH);
String expectedString = "Wed, 07 May 2014 17:43:26 GMT";
assertThat(string).isEqualTo(expectedString);
}

@Test
public void parseRfc822Date_DateWithTwoDigitDayOfMonth_ReturnsInstantObject() throws ParseException {
String formattedDate = LONG_DATE_FORMAT.format(Date.from(INSTANT));
Instant parsedInstant = DateUtils.parseRfc822Date(formattedDate);
assertThat(parsedInstant).isEqualTo(INSTANT);
}

@Test
public void parseRfc822Date_DateWithSingleDigitDayOfMonth_ReturnsInstantObject() throws ParseException {
final Instant INSTANT_SINGLE_DIGIT_DAY_OF_MONTH = Instant.ofEpochMilli(1399484606000L);;
String formattedDate = LONG_DATE_FORMAT.format(Date.from(INSTANT_SINGLE_DIGIT_DAY_OF_MONTH));
Instant parsedInstant = DateUtils.parseRfc822Date(formattedDate);
assertThat(parsedInstant).isEqualTo(INSTANT_SINGLE_DIGIT_DAY_OF_MONTH);
}

@Test
public void formatRfc1123Date() throws ParseException {
String string = DateUtils.formatRfc1123Date(INSTANT);
Expand All @@ -84,7 +123,7 @@ public void formatRfc1123Date() throws ParseException {
}

@Test
public void parseRfc822Date() throws ParseException {
public void parseRfc1123Date() throws ParseException {
String formatted = LONG_DATE_FORMAT.format(Date.from(INSTANT));
Instant expected = LONG_DATE_FORMAT.parse(formatted).toInstant();
Instant actual = DateUtils.parseRfc1123Date(formatted);
Expand Down