Skip to content

Commit

Permalink
Revert aggressively removing request content-type header when request…
Browse files Browse the repository at this point in the history
… body is empty, close #4509

Motivation:

As of #4429, we're aggressively removing request content-type header when request body is empty.
This is wrong, some content-types have valid empty string value, such as application/x-www-form-urlencoded.

Let's only not add a request content-type when using asJson and asXml when there's no body as an empty string is not a valid content for these encodings.

Modification:

Introduce HeadersBuiltIn that's only resolved when building the request.
  • Loading branch information
slandelle committed Jan 15, 2024
1 parent 888f430 commit 6e9b861
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -332,26 +332,6 @@ public T ignoreProtocolHeaders() {
return make(io.gatling.http.request.builder.RequestBuilder::ignoreProtocolHeaders);
}

/**
* Set the content-type header for JSON
*
* @return a new DSL instance
*/
@NonNull
public T asJson() {
return make(io.gatling.http.request.builder.RequestBuilder::asJson);
}

/**
* Set the content-type header for XML
*
* @return a new DSL instance
*/
@NonNull
public T asXml() {
return make(io.gatling.http.request.builder.RequestBuilder::asXml);
}

/**
* Set the authorization header for Basic Auth
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,24 @@ public T formUpload(
javaFunctionToExpression(filePath),
io.gatling.core.Predef.rawFileBodies()));
}

/**
* Set the content-type header for JSON
*
* @return a new DSL instance
*/
@NonNull
public T asJson() {
return make(io.gatling.http.request.builder.RequestWithBodyBuilder::asJson);
}

/**
* Set the content-type header for XML
*
* @return a new DSL instance
*/
@NonNull
public T asXml() {
return make(io.gatling.http.request.builder.RequestWithBodyBuilder::asXml);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ object HttpRequestExpressionBuilder {
value <- bodyPart.toMultiPart(session)
} yield accValue :+ value
}

private def filterOutCaseInsensitiveKey[T](map: Map[CharSequence, T], key: CharSequence): Map[CharSequence, T] = {
val keyString = key.toString
map.filterNot { case (key, _) => key.toString.equalsIgnoreCase(keyString) }
}
}

final class HttpRequestExpressionBuilder(
Expand All @@ -66,14 +61,13 @@ final class HttpRequestExpressionBuilder(
httpProtocol: HttpProtocol,
configuration: GatlingConfiguration
) extends RequestExpressionBuilder(commonAttributes, httpCaches, httpProtocol, configuration) {

require(bodyAttributes.body.isEmpty || bodyAttributes.bodyParts.isEmpty, "Can't have both a body and body parts!")

override final def sanitizeHeaders(rawHeaders: Map[CharSequence, Expression[String]]): Map[CharSequence, Expression[String]] =
if (bodyAttributes.body.isEmpty && bodyAttributes.bodyParts.isEmpty) {
// remove "content-type" that's only legit when there's a body
HttpRequestExpressionBuilder.filterOutCaseInsensitiveKey(rawHeaders, HttpHeaderNames.CONTENT_TYPE)
} else {
rawHeaders
override protected def configureHeaders(rawHeaders: Map[CharSequence, Expression[String]]): Map[CharSequence, Expression[String]] =
bodyAttributes.headersBuiltIn match {
case Some(headersBuiltIn) => headersBuiltIn.patch(rawHeaders, bodyAttributes.body.isEmpty && bodyAttributes.bodyParts.isEmpty)
case _ => rawHeaders
}

private def mergeFormParamsAndFormIntoParamJList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ object RequestBuilder {
HttpStatusCheckBuilder.find.validate(okStatusValidator.expressionSuccess).build(HttpStatusCheckMaterializer.Instance)
}

private val JsonHeaderValueExpression = HttpHeaderValues.APPLICATION_JSON.toString.expressionSuccess
private val XmlHeaderValueExpression = HttpHeaderValues.APPLICATION_XML.toString.expressionSuccess
val AcceptAllHeaderValueExpression: Expression[String] = "*/*".expressionSuccess
val AcceptCssHeaderValueExpression: Expression[String] = "text/css,*/*;q=0.1".expressionSuccess

Expand Down Expand Up @@ -147,18 +145,6 @@ abstract class RequestBuilder[B <: RequestBuilder[B]] {

def ignoreProtocolHeaders: B = newInstance(modify(commonAttributes)(_.ignoreProtocolHeaders).setTo(true))

/**
* Adds Accept and Content-Type headers to the request set with "application/json" values
*/
def asJson: B =
header(HttpHeaderNames.ACCEPT, RequestBuilder.JsonHeaderValueExpression).header(HttpHeaderNames.CONTENT_TYPE, RequestBuilder.JsonHeaderValueExpression)

/**
* Adds Accept and Content-Type headers to the request set with "application/xml" values
*/
def asXml: B =
header(HttpHeaderNames.ACCEPT, RequestBuilder.XmlHeaderValueExpression).header(HttpHeaderNames.CONTENT_TYPE, RequestBuilder.XmlHeaderValueExpression)

/**
* Adds BASIC authentication to the request
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ abstract class RequestExpressionBuilder(
mergeCaseInsensitive(uniqueProtocolHeaders, commonAttributes.headers)
}

sanitizeHeaders(rawHeaders)
configureHeaders(rawHeaders)
}

protected def sanitizeHeaders(rawHeaders: Map[CharSequence, Expression[String]]): Map[CharSequence, Expression[String]] =
protected def configureHeaders(rawHeaders: Map[CharSequence, Expression[String]]): Map[CharSequence, Expression[String]] =
rawHeaders

private val refererHeaderIsUndefined: Boolean = !headers.keys.exists(AsciiString.contentEqualsIgnoreCase(_, HttpHeaderNames.REFERER))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,44 @@
package io.gatling.http.request.builder

import io.gatling.core.body.{ Body, RawFileBodies }
import io.gatling.core.session.{ tupleSeq2SeqExpression, Expression, ExpressionSuccessWrapper }
import io.gatling.core.session._
import io.gatling.http.request.BodyPart

import com.softwaremill.quicklens.ModifyPimp
import com.softwaremill.quicklens._
import io.netty.handler.codec.http.{ HttpHeaderNames, HttpHeaderValues }

object BodyAttributes {
val Empty: BodyAttributes = BodyAttributes(body = None, bodyParts = Nil, formParams = Nil, form = None)
val Empty: BodyAttributes = BodyAttributes(body = None, bodyParts = Nil, formParams = Nil, form = None, headersBuiltIn = None)

sealed abstract class HeadersBuiltIn(contentHeaderValue: Expression[String]) {

private def containsCaseInsensitive(map: Map[CharSequence, _], key: CharSequence): Boolean =
map.keys.exists(_.toString.equalsIgnoreCase(key.toString))
def patch(rawHeaders: Map[CharSequence, Expression[String]], hasBody: Boolean): Map[CharSequence, Expression[String]] =
rawHeaders ++
(if (!containsCaseInsensitive(rawHeaders, HttpHeaderNames.ACCEPT)) {
Map(HttpHeaderNames.ACCEPT -> contentHeaderValue)
} else {
Map.empty
}) ++
(if (hasBody && !containsCaseInsensitive(rawHeaders, HttpHeaderNames.CONTENT_TYPE)) {
Map(HttpHeaderNames.CONTENT_TYPE -> contentHeaderValue)
} else {
Map.empty
})
}
object HeadersBuiltIn {
case object AsJson extends HeadersBuiltIn(HttpHeaderValues.APPLICATION_JSON.toString.expressionSuccess)
case object AsXml extends HeadersBuiltIn(HttpHeaderValues.APPLICATION_XML.toString.expressionSuccess)
}
}

final case class BodyAttributes(
body: Option[Body],
bodyParts: List[BodyPart],
formParams: List[HttpParam],
form: Option[Expression[Map[String, Any]]]
form: Option[Expression[Map[String, Any]]],
headersBuiltIn: Option[BodyAttributes.HeadersBuiltIn]
)

object RequestWithBodyBuilder {
Expand Down Expand Up @@ -84,4 +107,14 @@ abstract class RequestWithBodyBuilder[B <: RequestWithBodyBuilder[B]] extends Re

def formUpload(name: Expression[String], filePath: Expression[String])(implicit rawFileBodies: RawFileBodies): B =
bodyPart(BodyPart.rawFileBodyPart(Some(name), filePath, rawFileBodies))

/**
* Adds Accept and Content-Type headers to the request set with "application/json" values
*/
def asJson: B = newInstance(modify(bodyAttributes)(_.headersBuiltIn).setTo(Some(BodyAttributes.HeadersBuiltIn.AsJson)))

/**
* Adds Accept and Content-Type headers to the request set with "application/xml" values
*/
def asXml: B = newInstance(modify(bodyAttributes)(_.headersBuiltIn).setTo(Some(BodyAttributes.HeadersBuiltIn.AsXml)))
}

0 comments on commit 6e9b861

Please sign in to comment.