Skip to content

Commit

Permalink
Fix If-Unmodified-Since header mapping
Browse files Browse the repository at this point in the history
https://build.spring.io/browse/INT-MASTERSPRING40-664
https://build.spring.io/browse/INT-FATS5IC-833

Fix `DefaultHttpHeaderMapper` to populate an `If-Unmodified-Since`
request header with the same formatter as it is in the `HttpHeaders` in
Spring Web

**Cherry-pick to 5.1.x & 5.0.x**
  • Loading branch information
artembilan authored and garyrussell committed Apr 2, 2019
1 parent 23a73fa commit cb5d7f6
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 46 deletions.
Expand Up @@ -370,7 +370,7 @@ private HttpEntity<?> createHttpEntityFromPayload(Message<?> message, HttpMethod
// payload is already an HttpEntity, just return it as-is
return (HttpEntity<?>) payload;
}
HttpHeaders httpHeaders = this.mapHeaders(message);
HttpHeaders httpHeaders = mapHeaders(message);
if (!shouldIncludeRequestBody(httpMethod)) {
return new HttpEntity<>(httpHeaders);
}
Expand Down
Expand Up @@ -262,6 +262,10 @@ public class DefaultHttpHeaderMapper implements HeaderMapper<HttpHeaders>, BeanF
// Copy of 'org.springframework.http.HttpHeaders#GMT'
private static final ZoneId GMT = ZoneId.of("GMT");

// Copy of 'org.springframework.http.HttpHeaders#DATE_FORMATTER'
protected static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US).withZone(GMT);

// Copy of 'org.springframework.http.HttpHeaders#DATE_FORMATS'
protected static final DateTimeFormatter[] DATE_FORMATS = new DateTimeFormatter[] {
DateTimeFormatter.RFC_1123_DATE_TIME,
Expand Down Expand Up @@ -413,27 +417,28 @@ public void setUserDefinedHeaderPrefix(String userDefinedHeaderPrefix) {
@Override
public void fromHeaders(MessageHeaders headers, HttpHeaders target) {
if (this.logger.isDebugEnabled()) {
this.logger.debug(MessageFormat.format("outboundHeaderNames={0}",
CollectionUtils.arrayToList(this.outboundHeaderNames)));
this.logger.debug("outboundHeaderNames=" + Arrays.toString(this.outboundHeaderNames));
}
for (Entry<String, Object> entry : headers.entrySet()) {
String name = entry.getKey();
String lowerName = name.toLowerCase();
if (this.shouldMapOutboundHeader(lowerName)) {
if (shouldMapOutboundHeader(lowerName)) {
Object value = entry.getValue();
if (value != null) {
if (!HTTP_REQUEST_HEADER_NAMES_LOWER.contains(lowerName) &&
!HTTP_RESPONSE_HEADER_NAMES_LOWER.contains(lowerName) &&
!MessageHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) {
// prefix the user-defined header names if not already prefixed

name = StringUtils.startsWithIgnoreCase(name, this.userDefinedHeaderPrefix) ? name :
this.userDefinedHeaderPrefix + name;
name =
StringUtils.startsWithIgnoreCase(name, this.userDefinedHeaderPrefix)
? name
: this.userDefinedHeaderPrefix + name;
}
if (this.logger.isDebugEnabled()) {
this.logger.debug(MessageFormat.format("setting headerName=[{0}], value={1}", name, value));
}
this.setHttpHeader(target, name, value);
setHttpHeader(target, name, value);
}
}
}
Expand All @@ -450,36 +455,36 @@ public Map<String, Object> toHeaders(HttpHeaders source) {
this.logger.debug(MessageFormat.format("inboundHeaderNames={0}",
CollectionUtils.arrayToList(this.inboundHeaderNames)));
}
Map<String, Object> target = new HashMap<String, Object>();
Map<String, Object> target = new HashMap<>();
Set<String> headerNames = source.keySet();
for (String name : headerNames) {
String lowerName = name.toLowerCase();
if (this.shouldMapInboundHeader(lowerName)) {
if (shouldMapInboundHeader(lowerName)) {
if (!HTTP_REQUEST_HEADER_NAMES_LOWER.contains(lowerName)
&& !HTTP_RESPONSE_HEADER_NAMES_LOWER.contains(lowerName)) {
String prefixedName = StringUtils.startsWithIgnoreCase(name, this.userDefinedHeaderPrefix)
? name
: this.userDefinedHeaderPrefix + name;
Object value = source.containsKey(prefixedName)
? this.getHttpHeader(source, prefixedName)
: this.getHttpHeader(source, name);
? getHttpHeader(source, prefixedName)
: getHttpHeader(source, name);
if (value != null) {
if (this.logger.isDebugEnabled()) {
this.logger.debug(MessageFormat.format("setting headerName=[{0}], value={1}", name, value));
}
this.setMessageHeader(target, name, value);
setMessageHeader(target, name, value);
}
}
else {
Object value = this.getHttpHeader(source, name);
Object value = getHttpHeader(source, name);
if (value != null) {
if (this.logger.isDebugEnabled()) {
this.logger.debug(MessageFormat.format("setting headerName=[{0}], value={1}", name, value));
}
if (CONTENT_TYPE.equalsIgnoreCase(name)) {
name = MessageHeaders.CONTENT_TYPE;
}
this.setMessageHeader(target, name, value);
setMessageHeader(target, name, value);
}
}
}
Expand Down Expand Up @@ -511,9 +516,10 @@ private boolean shouldMapOutboundHeader(String headerName) {
* When using the default response header name list, suppress the
* mapping of exclusions for specific headers.
*/
if (this.containsElementIgnoreCase(this.excludedInboundStandardResponseHeaderNames, headerName)) {
if (containsElementIgnoreCase(this.excludedInboundStandardResponseHeaderNames, headerName)) {
if (this.logger.isDebugEnabled()) {
this.logger.debug(MessageFormat.format("headerName=[{0}] WILL NOT be mapped (excluded)", headerName));
this.logger
.debug(MessageFormat.format("headerName=[{0}] WILL NOT be mapped (excluded)", headerName));
}
return false;
}
Expand All @@ -524,18 +530,19 @@ else if (this.isDefaultOutboundMapper) {
* When using the default request header name list, suppress the
* mapping of exclusions for specific headers.
*/
if (this.containsElementIgnoreCase(this.excludedOutboundStandardRequestHeaderNames, headerName)) {
if (containsElementIgnoreCase(this.excludedOutboundStandardRequestHeaderNames, headerName)) {
if (this.logger.isDebugEnabled()) {
this.logger.debug(MessageFormat.format("headerName=[{0}] WILL NOT be mapped (excluded)", headerName));
this.logger
.debug(MessageFormat.format("headerName=[{0}] WILL NOT be mapped (excluded)", headerName));
}
return false;
}
}
return this.shouldMapHeader(headerName, outboundHeaderNamesLower);
return shouldMapHeader(headerName, outboundHeaderNamesLower);
}

protected final boolean shouldMapInboundHeader(String headerName) {
return this.shouldMapHeader(headerName, this.inboundHeaderNamesLower);
return shouldMapHeader(headerName, this.inboundHeaderNamesLower);
}

/**
Expand Down Expand Up @@ -582,7 +589,7 @@ private void setHttpHeader(HttpHeaders target, String name, Object value) {
if (value instanceof Collection<?>) {
Collection<?> values = (Collection<?>) value;
if (!CollectionUtils.isEmpty(values)) {
List<MediaType> acceptableMediaTypes = new ArrayList<MediaType>();
List<MediaType> acceptableMediaTypes = new ArrayList<>();
for (Object type : values) {
if (type instanceof MediaType) {
acceptableMediaTypes.add((MediaType) type);
Expand All @@ -604,7 +611,7 @@ else if (value instanceof MediaType) {
target.setAccept(Collections.singletonList((MediaType) value));
}
else if (value instanceof String[]) {
List<MediaType> acceptableMediaTypes = new ArrayList<MediaType>();
List<MediaType> acceptableMediaTypes = new ArrayList<>();
for (String next : (String[]) value) {
acceptableMediaTypes.add(MediaType.parseMediaType(next));
}
Expand All @@ -623,7 +630,7 @@ else if (ACCEPT_CHARSET.equalsIgnoreCase(name)) {
if (value instanceof Collection<?>) {
Collection<?> values = (Collection<?>) value;
if (!CollectionUtils.isEmpty(values)) {
List<Charset> acceptableCharsets = new ArrayList<Charset>();
List<Charset> acceptableCharsets = new ArrayList<>();
for (Object charset : values) {
if (charset instanceof Charset) {
acceptableCharsets.add((Charset) charset);
Expand All @@ -642,7 +649,7 @@ else if (charset instanceof String) {
}
}
else if (value instanceof Charset[] || value instanceof String[]) {
List<Charset> acceptableCharsets = new ArrayList<Charset>();
List<Charset> acceptableCharsets = new ArrayList<>();
Object[] values = ObjectUtils.toObjectArray(value);
for (Object charset : values) {
if (charset instanceof Charset) {
Expand All @@ -659,7 +666,7 @@ else if (value instanceof Charset) {
}
else if (value instanceof String) {
String[] charsets = StringUtils.commaDelimitedListToStringArray((String) value);
List<Charset> acceptableCharsets = new ArrayList<Charset>();
List<Charset> acceptableCharsets = new ArrayList<>();
for (String charset : charsets) {
acceptableCharsets.add(Charset.forName(charset.trim()));
}
Expand All @@ -675,7 +682,7 @@ else if (ALLOW.equalsIgnoreCase(name)) {
if (value instanceof Collection<?>) {
Collection<?> values = (Collection<?>) value;
if (!CollectionUtils.isEmpty(values)) {
Set<HttpMethod> allowedMethods = new HashSet<HttpMethod>();
Set<HttpMethod> allowedMethods = new HashSet<>();
for (Object method : values) {
if (method instanceof HttpMethod) {
allowedMethods.add((HttpMethod) method);
Expand All @@ -698,14 +705,14 @@ else if (method instanceof String) {
target.setAllow(Collections.singleton((HttpMethod) value));
}
else if (value instanceof HttpMethod[]) {
Set<HttpMethod> allowedMethods = new HashSet<HttpMethod>();
Set<HttpMethod> allowedMethods = new HashSet<>();
Collections.addAll(allowedMethods, (HttpMethod[]) value);
target.setAllow(allowedMethods);
}
else if (value instanceof String || value instanceof String[]) {
String[] values = (value instanceof String[]) ? (String[]) value
: StringUtils.commaDelimitedListToStringArray((String) value);
Set<HttpMethod> allowedMethods = new HashSet<HttpMethod>();
Set<HttpMethod> allowedMethods = new HashSet<>();
for (String next : values) {
allowedMethods.add(HttpMethod.valueOf(next.trim()));
}
Expand Down Expand Up @@ -766,7 +773,7 @@ else if (value instanceof String) {
target.setDate(Long.parseLong((String) value));
}
catch (@SuppressWarnings(UNUSED) NumberFormatException e) {
target.setDate(this.getFirstDate((String) value, DATE));
target.setDate(getFirstDate((String) value, DATE));
}
}
else {
Expand Down Expand Up @@ -797,7 +804,7 @@ else if (value instanceof String) {
target.setExpires(Long.parseLong((String) value));
}
catch (@SuppressWarnings(UNUSED) NumberFormatException e) {
target.setExpires(this.getFirstDate((String) value, EXPIRES));
target.setExpires(getFirstDate((String) value, EXPIRES));
}
}
else {
Expand All @@ -818,7 +825,7 @@ else if (value instanceof String) {
target.setIfModifiedSince(Long.parseLong((String) value));
}
catch (@SuppressWarnings(UNUSED) NumberFormatException e) {
target.setIfModifiedSince(this.getFirstDate((String) value, IF_MODIFIED_SINCE));
target.setIfModifiedSince(getFirstDate((String) value, IF_MODIFIED_SINCE));
}
}
else {
Expand All @@ -829,20 +836,20 @@ else if (value instanceof String) {
}
}
else if (IF_UNMODIFIED_SINCE.equalsIgnoreCase(name)) {
String ifUnmodifiedSinceValue = null;
String ifUnmodifiedSinceValue;
if (value instanceof Date) {
ifUnmodifiedSinceValue = this.formatDate(((Date) value).getTime());
ifUnmodifiedSinceValue = formatDate(((Date) value).getTime());
}
else if (value instanceof Number) {
ifUnmodifiedSinceValue = this.formatDate(((Number) value).longValue());
ifUnmodifiedSinceValue = formatDate(((Number) value).longValue());
}
else if (value instanceof String) {
try {
ifUnmodifiedSinceValue = this.formatDate(Long.parseLong((String) value));
ifUnmodifiedSinceValue = formatDate(Long.parseLong((String) value));
}
catch (@SuppressWarnings(UNUSED) NumberFormatException e) {
long longValue = this.getFirstDate((String) value, IF_UNMODIFIED_SINCE);
ifUnmodifiedSinceValue = this.formatDate(longValue);
long longValue = getFirstDate((String) value, IF_UNMODIFIED_SINCE);
ifUnmodifiedSinceValue = formatDate(longValue);
}
}
else {
Expand All @@ -864,7 +871,7 @@ else if (value instanceof String[]) {
else if (value instanceof Collection) {
Collection<?> values = (Collection<?>) value;
if (!CollectionUtils.isEmpty(values)) {
List<String> ifNoneMatchList = new ArrayList<String>();
List<String> ifNoneMatchList = new ArrayList<>();
for (Object next : values) {
if (next instanceof String) {
ifNoneMatchList.add((String) next);
Expand All @@ -891,7 +898,7 @@ else if (value instanceof String) {
target.setLastModified(Long.parseLong((String) value));
}
catch (@SuppressWarnings(UNUSED) NumberFormatException e) {
target.setLastModified(this.getFirstDate((String) value, LAST_MODIFIED));
target.setLastModified(getFirstDate((String) value, LAST_MODIFIED));
}
}
else {
Expand Down Expand Up @@ -939,12 +946,12 @@ else if (value instanceof String[]) {
}
else if (value instanceof Iterable<?>) {
for (Object next : (Iterable<?>) value) {
String convertedValue = null;
String convertedValue;
if (next instanceof String) {
convertedValue = (String) next;
}
else {
convertedValue = this.convertToString(value);
convertedValue = convertToString(value);
}
if (StringUtils.hasText(convertedValue)) {
target.add(name, convertedValue);
Expand All @@ -957,7 +964,7 @@ else if (value instanceof Iterable<?>) {
}
}
else {
String convertedValue = this.convertToString(value);
String convertedValue = convertToString(value);
if (StringUtils.hasText(convertedValue)) {
target.set(name, convertedValue);
}
Expand Down Expand Up @@ -1018,7 +1025,7 @@ else if (IF_MODIFIED_SINCE.equalsIgnoreCase(name)) {
}
else if (IF_UNMODIFIED_SINCE.equalsIgnoreCase(name)) {
String unmodifiedSince = source.getFirst(IF_UNMODIFIED_SINCE);
return unmodifiedSince != null ? this.getFirstDate(unmodifiedSince, IF_UNMODIFIED_SINCE) : null;
return unmodifiedSince != null ? getFirstDate(unmodifiedSince, IF_UNMODIFIED_SINCE) : null;
}
else if (LAST_MODIFIED.equalsIgnoreCase(name)) {
long lastModified = source.getLastModified();
Expand Down Expand Up @@ -1066,6 +1073,7 @@ protected String convertToString(Object value) {
if (this.conversionService != null &&
this.conversionService.canConvert(TypeDescriptor.forObject(value),
TypeDescriptor.valueOf(String.class))) {

return this.conversionService.convert(value, String.class);
}
return null;
Expand All @@ -1089,10 +1097,10 @@ protected long getFirstDate(String headerValue, String headerName) {
+ "' header");
}

protected String formatDate(long date) {
protected static String formatDate(long date) {
Instant instant = Instant.ofEpochMilli(date);
ZonedDateTime zonedDateTime = ZonedDateTime.ofInstant(instant, GMT);
return DATE_FORMATS[0].format(zonedDateTime);
return DATE_FORMATTER.format(zonedDateTime);
}

/**
Expand Down
Expand Up @@ -190,7 +190,7 @@ public void testHttpMultipartProxyScenario() throws Exception {
MultiValueMap<String, String> responseHeaders = new LinkedMultiValueMap<>(httpHeaders);
responseHeaders.set("Connection", "close");
responseHeaders.set("Content-Type", "text/plain");
return new ResponseEntity<Object>(responseHeaders, HttpStatus.OK);
return new ResponseEntity<>(responseHeaders, HttpStatus.OK);
}).when(template).exchange(Mockito.any(URI.class), Mockito.any(HttpMethod.class),
Mockito.any(HttpEntity.class), (Class<?>) isNull());

Expand Down

0 comments on commit cb5d7f6

Please sign in to comment.