{"action":"created","issue":{"url":"https://api.github.com/repos/lagom/lagom/issues/1030","repository_url":"https://api.github.com/repos/lagom/lagom","labels_url":"https://api.github.com/repos/lagom/lagom/issues/1030/labels{/name}","comments_url":"https://api.github.com/repos/lagom/lagom/issues/1030/comments","events_url":"https://api.github.com/repos/lagom/lagom/issues/1030/events","html_url":"https://github.com/lagom/lagom/pull/1030","id":263112105,"number":1030,"title":"[WIP] Service Router actions should used filtered request","user":{"login":"ignasi35","id":762126,"avatar_url":"https://avatars2.githubusercontent.com/u/762126?v=4","gravatar_id":"","url":"https://api.github.com/users/ignasi35","html_url":"https://github.com/ignasi35","followers_url":"https://api.github.com/users/ignasi35/followers","following_url":"https://api.github.com/users/ignasi35/following{/other_user}","gists_url":"https://api.github.com/users/ignasi35/gists{/gist_id}","starred_url":"https://api.github.com/users/ignasi35/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ignasi35/subscriptions","organizations_url":"https://api.github.com/users/ignasi35/orgs","repos_url":"https://api.github.com/users/ignasi35/repos","events_url":"https://api.github.com/users/ignasi35/events{/privacy}","received_events_url":"https://api.github.com/users/ignasi35/received_events","type":"User","site_admin":false},"labels":[{"id":597655013,"url":"https://api.github.com/repos/lagom/lagom/labels/status:needs-backport","name":"status:needs-backport","color":"fbca04","default":false},{"id":660037261,"url":"https://api.github.com/repos/lagom/lagom/labels/topic:service-api","name":"topic:service-api","color":"c5def5","default":false},{"id":330385907,"url":"https://api.github.com/repos/lagom/lagom/labels/type:defect","name":"type:defect","color":"d93f0b","default":false}],"state":"open","locked":false,"assignee":{"login":"ignasi35","id":762126,"avatar_url":"https://avatars2.githubusercontent.com/u/762126?v=4","gravatar_id":"","url":"https://api.github.com/users/ignasi35","html_url":"https://github.com/ignasi35","followers_url":"https://api.github.com/users/ignasi35/followers","following_url":"https://api.github.com/users/ignasi35/following{/other_user}","gists_url":"https://api.github.com/users/ignasi35/gists{/gist_id}","starred_url":"https://api.github.com/users/ignasi35/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ignasi35/subscriptions","organizations_url":"https://api.github.com/users/ignasi35/orgs","repos_url":"https://api.github.com/users/ignasi35/repos","events_url":"https://api.github.com/users/ignasi35/events{/privacy}","received_events_url":"https://api.github.com/users/ignasi35/received_events","type":"User","site_admin":false},"assignees":[{"login":"ignasi35","id":762126,"avatar_url":"https://avatars2.githubusercontent.com/u/762126?v=4","gravatar_id":"","url":"https://api.github.com/users/ignasi35","html_url":"https://github.com/ignasi35","followers_url":"https://api.github.com/users/ignasi35/followers","following_url":"https://api.github.com/users/ignasi35/following{/other_user}","gists_url":"https://api.github.com/users/ignasi35/gists{/gist_id}","starred_url":"https://api.github.com/users/ignasi35/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ignasi35/subscriptions","organizations_url":"https://api.github.com/users/ignasi35/orgs","repos_url":"https://api.github.com/users/ignasi35/repos","events_url":"https://api.github.com/users/ignasi35/events{/privacy}","received_events_url":"https://api.github.com/users/ignasi35/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/lagom/lagom/milestones/22","html_url":"https://github.com/lagom/lagom/milestone/22","labels_url":"https://api.github.com/repos/lagom/lagom/milestones/22/labels","id":2813770,"number":22,"title":"Lagom 1.3.10","description":"","creator":{"login":"rcavalcanti","id":502982,"avatar_url":"https://avatars0.githubusercontent.com/u/502982?v=4","gravatar_id":"","url":"https://api.github.com/users/rcavalcanti","html_url":"https://github.com/rcavalcanti","followers_url":"https://api.github.com/users/rcavalcanti/followers","following_url":"https://api.github.com/users/rcavalcanti/following{/other_user}","gists_url":"https://api.github.com/users/rcavalcanti/gists{/gist_id}","starred_url":"https://api.github.com/users/rcavalcanti/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rcavalcanti/subscriptions","organizations_url":"https://api.github.com/users/rcavalcanti/orgs","repos_url":"https://api.github.com/users/rcavalcanti/repos","events_url":"https://api.github.com/users/rcavalcanti/events{/privacy}","received_events_url":"https://api.github.com/users/rcavalcanti/received_events","type":"User","site_admin":false},"open_issues":3,"closed_issues":0,"state":"open","created_at":"2017-10-04T08:23:20Z","updated_at":"2017-10-06T07:58:11Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2017-10-05T12:46:48Z","updated_at":"2017-10-09T17:50:19Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/lagom/lagom/pulls/1030","html_url":"https://github.com/lagom/lagom/pull/1030","diff_url":"https://github.com/lagom/lagom/pull/1030.diff","patch_url":"https://github.com/lagom/lagom/pull/1030.patch"},"body":"When a new request arrives, the `ServiceRouter` in Lagom builds a Play `Action` on the fly. That `Action` closes over the `request.headers` as received over the wire before the request is filtered.\r\n\r\nThe side-effect of that is that when using Play filters in a Lagom service, if the play filter modifies the headers then the modifications are not available on the Lagom code. Other modifications on the request are available on the Lagom code because the `Action` build on the fly by the `ServiceRouter` distinguishes between `request` and `headers`.\r\n\r\nThis PR ignores the headers received in favour of parsing the headers only when they are needed."},"comment":{"url":"https://api.github.com/repos/lagom/lagom/issues/comments/335232988","html_url":"https://github.com/lagom/lagom/pull/1030#issuecomment-335232988","issue_url":"https://api.github.com/repos/lagom/lagom/issues/1030","id":335232988,"user":{"login":"ignasi35","id":762126,"avatar_url":"https://avatars2.githubusercontent.com/u/762126?v=4","gravatar_id":"","url":"https://api.github.com/users/ignasi35","html_url":"https://github.com/ignasi35","followers_url":"https://api.github.com/users/ignasi35/followers","following_url":"https://api.github.com/users/ignasi35/following{/other_user}","gists_url":"https://api.github.com/users/ignasi35/gists{/gist_id}","starred_url":"https://api.github.com/users/ignasi35/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ignasi35/subscriptions","organizations_url":"https://api.github.com/users/ignasi35/orgs","repos_url":"https://api.github.com/users/ignasi35/repos","events_url":"https://api.github.com/users/ignasi35/events{/privacy}","received_events_url":"https://api.github.com/users/ignasi35/received_events","type":"User","site_admin":false},"created_at":"2017-10-09T17:50:19Z","updated_at":"2017-10-09T17:50:19Z","author_association":"CONTRIBUTOR","body":"> It looks like websocket handling bypasses Play's request handling entirely. Is that correct? Does that imply that Play filters will not be invoked at all for streaming services? If that's the case, I don't think we should recommend using them for things such as CORS and authentication handling.\r\n\r\nIIUC play is used in [this call](https://github.com/lagom/lagom/blob/dfabd67976e4d2c070f643b8ca9b9089f6fb2a9b/service/core/server/src/main/scala/com/lightbend/lagom/internal/server/ServiceRouter.scala#L262). I had to check the [play docs](https://www.playframework.com/documentation/2.6.x/ScalaWebSockets#Rejecting-a-WebSocket) to understand that bit.\r\n\r\n\r\n> The request header parsing is a little convoluted (already before this change) and it looks like it will now call toRequestHeader twice per (strict) call. The existing call at the top of routes is still there, but it is now throwing away most of the work that it does. It seems like this could be changed to pull the request path directly from the Play request without converting it first.\r\n\r\nI've split the process of converting the headers into the lagom class and applying the filters. This way we have a better control of when each operation happens.\r\n\r\n> Can we have an automated test for this?\r\n\r\nDone for strict messages in scaladsl. Will try to implement for websocket and javadsl too (hopefully copy/paste and package renaming) but an early review of the code as is will be helpful."}}
{"actor":{"display_login":"ignasi35"}}