HTTPRequest type
I believe the HTTPRequest
type in Internet.HTTP
could use some reconsideration. It currently looks like this:
:: HTTPRequest = { req_method :: HTTPMethod // The HTTP request method (eg. GET, POST, HEAD)
, req_path :: String // The requested location (eg. /foo)
, req_query :: String // The query part of a location (eg. ?foo=bar&baz=42)
, req_version :: String // The http version (eg. HTTP/1.0 or HTTP/1.1)
, req_protocol :: HTTPProtocol // Protocol info, http or https
, req_headers :: Map String String // The headers sent with the request parsed into name/value pairs
, req_data :: String // The raw data of the request (without the headers)
, arg_get :: Map String String // The arguments passed in the url
, arg_post :: Map String String // The arguments passed via the POST method
, arg_cookies :: Map String String // The cookies in the set-cookie header
, arg_uploads :: Map String HTTPUpload // Uploads that are sent via the POST method
, server_name :: String // Server host name or ip address
, server_port :: Int // Server port
, client_name :: String // Client host name or ip address
}
There are several oddities here:
- The
arg_*
fields are used in some places, but for instance not in thetoString
instance and therefore they are not considered bydoHTTPRequest
. It is also not clear howarg_get
andreq_query
relate to each other or what happens whenarg_post
is non-empty but thereq_method
is notHTTP_POST
, etc. I suggest these fields are removed from here; perhaps functions likegetUploads :: HTTPRequest -> Map String HTTPUpload
can be added that parse the uploads from an existing request, and, conversely,addUploads :: (Map String HTTPUpload) HTTPRequest -> HTTPRequest
. -
client_name
andserver_port
are as far as I know not part of a request message as defined in RFC 2616, so it seems odd to me that they are included in this type. And why, for instance, there is noclient_port
. Perhaps these fields could be moved to a newTCPMessageDetails
type or so. This influences iTasks.Extensions.Web, iTasks.Internal.HttpUtil, iTasks.Internal.WebService and Internet.HTTP.CGI. - Similarly,
req_protocol
does not really 'belong' to an HTTP request either. It is not used anywhere. I suggest removing this and theHTTPProtocol
type because this suggests thatdoRequest
is capable of handling SSL. - Many servers reply to HTTP/1.1 requests with chunked transfer-coding. However, the
doHTTPRequest
function is not able to handle this. So to have thereq_version
field in the record gives an incorrect idea of the capabilities of the library. HTTP/1.1 is checked for in iTasks.Extensions.Web and iTasks.Internal.WebService to check for keep-alive options.
Some of these comments also apply to the HTTPResponse
type. Additionally, it is odd that the headers are there stored as a list but as a Map in the request type.