{"action":"opened","issue":{"url":"https://api.github.com/repos/SuaveIO/suave/issues/229","labels_url":"https://api.github.com/repos/SuaveIO/suave/issues/229/labels{/name}","comments_url":"https://api.github.com/repos/SuaveIO/suave/issues/229/comments","events_url":"https://api.github.com/repos/SuaveIO/suave/issues/229/events","html_url":"https://github.com/SuaveIO/suave/issues/229","id":66627423,"number":229,"title":"Consider how we parse request data","user":{"login":"haf","id":193115,"avatar_url":"https://avatars.githubusercontent.com/u/193115?v=3","gravatar_id":"","url":"https://api.github.com/users/haf","html_url":"https://github.com/haf","followers_url":"https://api.github.com/users/haf/followers","following_url":"https://api.github.com/users/haf/following{/other_user}","gists_url":"https://api.github.com/users/haf/gists{/gist_id}","starred_url":"https://api.github.com/users/haf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haf/subscriptions","organizations_url":"https://api.github.com/users/haf/orgs","repos_url":"https://api.github.com/users/haf/repos","events_url":"https://api.github.com/users/haf/events{/privacy}","received_events_url":"https://api.github.com/users/haf/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2015-04-06T15:18:13Z","updated_at":"2015-04-06T15:18:13Z","closed_at":null,"body":"Right now it's fairly ad-hoc. I would feel safer if we could use a parser like FParsec instead of hand-coded logic to find boundaries etc. Right now we have multipart/form-data that's silently blocking a connection based on input; if talking about DoS attacks like we did before; just browsing the code and tricking it into throwing an exception is enough to make requests never return.\r\n\r\nExample code:\r\n\r\n``` fsharp\r\n      if not(firstLine.Equals(\"--\")) then\r\n        let! partHeaders, connection = readHeaders connection\r\n        let contentDisposition = partHeaders %% \"content-disposition\"\r\n        let fieldname = \r\n          (headerParams contentDisposition).TryLookup \"name\"\r\n          |> Option.get\r\n          |> (fun x -> x.Trim('\"'))\r\n        let contentType = partHeaders %% \"content-type\"\r\n        match contentType with\r\n        | Some(x) when x.StartsWith(\"multipart/mixed\") ->\r\n          let subboundary = \"--\" + x.Substring(x.IndexOf('=') + 1).TrimStart()\r\n          return! loop subboundary { ctx with connection = connection }\r\n        | Some(x) ->\r\n          let tempFileName = Path.GetTempFileName()\r\n          use tempFile = new FileStream(tempFileName, FileMode.Truncate)\r\n          let! a, connection = readUntil (ASCII.bytes(eol + boundary)) (fun x y -> async { do! tempFile.AsyncWrite(x.Array, x.Offset, y) } ) connection\r\n          let fileLength = tempFile.Length\r\n          tempFile.Close()\r\n          if fileLength > 0L then\r\n            let filename =\r\n              (headerParams contentDisposition).[\"filename\"].Trim('\"')\r\n            let upload = \r\n                { fieldName     = fieldname\r\n                  fileName      = filename\r\n                  mimeType      = (contentType |> Option.get) \r\n                  tempFilePath = tempFileName }\r\n            return! loop boundary { ctx with request = { ctx.request with files = upload :: ctx.request.files};  connection = connection }\r\n          else\r\n            File.Delete tempFileName\r\n            return! loop boundary { ctx with connection = connection }\r\n```\r\n\r\nThis code has these concerns:\r\n\r\n - parsing by large pieces of arrow-code (if not ( ... ))\r\n - parsing by bytes\r\n - temporary file handling\r\n - stream IO to disk/buffering\r\n - taking finished parse data and transforming into DTOs\r\n - content-type matching\r\n\r\nSome patterns:\r\n\r\n - `let! a, connection = readUntil (ASCII.bytes(eol + boundary)) (fun x y -> async { do! tempFile.AsyncWrite(x.Array, x.Offset, y) } ) connection` is repeated twice\r\n\r\n\r\nWhat if?\r\n\r\n - Content-Type: multipart/form-data; charset=utf-16 was on the request was UTF-16; each character will now be four bytes, but this parsing logic assumes it's ASCII/one byte: `ASCII.toStringAtOffset byts 0 byts.Length`\r\n - If I send a file named: `Hello\\\" World.txt`?\r\n - I want to unit-test the parsing logic? I can't, because statics are used to access the FileSystem. And because it's bound inside the Socket, I have to create a real socket to test the algorithm that operates on purely in-memory data\r\n - I send base64-encoded data, which is valid according to the spec\r\n - I send a multipart/mixed and then, according to the spec don't have a fieldname for every uploaded file (that would crash here: `(headerParams contentDisposition).TryLookup \"name\" |> Option.get`\r\n - I use the header: `Content-Transfer-Encoding` and encode part of the request with e.g. IBM487 or Latin1, inside the form-data?"}}