{"action":"opened","number":754,"pull_request":{"url":"https://api.github.com/repos/C5T/Current/pulls/754","id":125760019,"html_url":"https://github.com/C5T/Current/pull/754","diff_url":"https://github.com/C5T/Current/pull/754.diff","patch_url":"https://github.com/C5T/Current/pull/754.patch","issue_url":"https://api.github.com/repos/C5T/Current/issues/754","number":754,"state":"open","locked":false,"title":"Blocks/HTTP/impl/posix_client: Fixed following redirect from a URL with non-default port to a full URL without a port.","user":{"login":"sompylasar","id":498274,"avatar_url":"https://avatars1.githubusercontent.com/u/498274?v=3","gravatar_id":"","url":"https://api.github.com/users/sompylasar","html_url":"https://github.com/sompylasar","followers_url":"https://api.github.com/users/sompylasar/followers","following_url":"https://api.github.com/users/sompylasar/following{/other_user}","gists_url":"https://api.github.com/users/sompylasar/gists{/gist_id}","starred_url":"https://api.github.com/users/sompylasar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sompylasar/subscriptions","organizations_url":"https://api.github.com/users/sompylasar/orgs","repos_url":"https://api.github.com/users/sompylasar/repos","events_url":"https://api.github.com/users/sompylasar/events{/privacy}","received_events_url":"https://api.github.com/users/sompylasar/received_events","type":"User","site_admin":false},"body":"When a redirect occurs from a URL with a port (\"previous URL\") to a URL without a port, the port was propagated to the resulting URL and to the \"Location\" header.\r\nExample:\r\n```\r\ntest.cc:156: Failure\r\nValue of: URL(\"ftp://host_no_port_path\", URL(\"meh://localhost:8080\")).ComposeURL()\r\n  Actual: \"ftp://host_no_port_path:8080/\"\r\nExpected: \"ftp://host_no_port_path/\"\r\ntest.cc:157: Failure\r\nValue of: URL(\"blah://new_host/foo\", URL(\"meh://localhost:5000\")).ComposeURL()\r\n  Actual: \"blah://new_host:5000/foo\"\r\nExpected: \"blah://new_host/foo\"\r\n```\r\n\r\nThe redirect URL transformation logic was implemented in the `URL` class constructor.\r\nIt was so multi-purpose (both for redirects and for assigning default values) so that there was no easy way to reason about it when fixing this port propagation issue and keep the rest of test cases green.\r\nThe multi-inheritance structure of the `URL` class added more to that spaghetti.\r\nThe above led to the decision to simplify the `URL` class public API and internal implementation.\r\n* Removed extra parameters to reduce the API surface that requires support and testing.\r\n* Simplified internal logic (it's still complex and hard to grasp, unfortunately, because we chose to support many partial URL formats and make it smart).\r\n* Made explicit static functions for redirect handling and for filling with default values.\r\n* Simplified `URL` class inheritance, using static functions, not constructors/initializers, to easier propagate data from one to another without a need for a class field (removed `url_without_parameters` field).\r\n* Instead of public `url_without_parameters` field, added `ComposeURLWithoutParameters` method because the field value is hard to keep in sync with the other properties' values (the scheme, host, port).\r\n* Added `ParseQueryString` static function to label that intent where needed (see `Midichlorians/Server/server.h`).\r\n\r\nThe full stack of the redirect with HTTP server and client cannot be tested with the Current tests setup for now because the spawned servers always use ports and cannot redirect to a URL without a port because `HTTPClient` tries to connect to it. There is a way to test the actual use case: to bind to the port `80`, but this requires root access. I've implemented this test and checked with root access but disabled it with `#if 0` for now.","created_at":"2017-06-15T07:42:16Z","updated_at":"2017-06-15T07:42:16Z","closed_at":null,"merged_at":null,"merge_commit_sha":null,"assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/C5T/Current/pulls/754/commits","review_comments_url":"https://api.github.com/repos/C5T/Current/pulls/754/comments","review_comment_url":"https://api.github.com/repos/C5T/Current/pulls/comments{/number}","comments_url":"https://api.github.com/repos/C5T/Current/issues/754/comments","statuses_url":"https://api.github.com/repos/C5T/Current/statuses/eef1ffc0a66d57149693bb76625669aac9579775","head":{"label":"sompylasar:blocks_http_client_redirect_fix_url_refactor","ref":"blocks_http_client_redirect_fix_url_refactor","sha":"eef1ffc0a66d57149693bb76625669aac9579775","user":{"login":"sompylasar","id":498274,"avatar_url":"https://avatars1.githubusercontent.com/u/498274?v=3","gravatar_id":"","url":"https://api.github.com/users/sompylasar","html_url":"https://github.com/sompylasar","followers_url":"https://api.github.com/users/sompylasar/followers","following_url":"https://api.github.com/users/sompylasar/following{/other_user}","gists_url":"https://api.github.com/users/sompylasar/gists{/gist_id}","starred_url":"https://api.github.com/users/sompylasar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sompylasar/subscriptions","organizations_url":"https://api.github.com/users/sompylasar/orgs","repos_url":"https://api.github.com/users/sompylasar/repos","events_url":"https://api.github.com/users/sompylasar/events{/privacy}","received_events_url":"https://api.github.com/users/sompylasar/received_events","type":"User","site_admin":false},"repo":{"id":29994593,"name":"Current","full_name":"sompylasar/Current","owner":{"login":"sompylasar","id":498274,"avatar_url":"https://avatars1.githubusercontent.com/u/498274?v=3","gravatar_id":"","url":"https://api.github.com/users/sompylasar","html_url":"https://github.com/sompylasar","followers_url":"https://api.github.com/users/sompylasar/followers","following_url":"https://api.github.com/users/sompylasar/following{/other_user}","gists_url":"https://api.github.com/users/sompylasar/gists{/gist_id}","starred_url":"https://api.github.com/users/sompylasar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sompylasar/subscriptions","organizations_url":"https://api.github.com/users/sompylasar/orgs","repos_url":"https://api.github.com/users/sompylasar/repos","events_url":"https://api.github.com/users/sompylasar/events{/privacy}","received_events_url":"https://api.github.com/users/sompylasar/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/sompylasar/Current","description":"C++ framework for realtime machine learning.","fork":true,"url":"https://api.github.com/repos/sompylasar/Current","forks_url":"https://api.github.com/repos/sompylasar/Current/forks","keys_url":"https://api.github.com/repos/sompylasar/Current/keys{/key_id}","collaborators_url":"https://api.github.com/repos/sompylasar/Current/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/sompylasar/Current/teams","hooks_url":"https://api.github.com/repos/sompylasar/Current/hooks","issue_events_url":"https://api.github.com/repos/sompylasar/Current/issues/events{/number}","events_url":"https://api.github.com/repos/sompylasar/Current/events","assignees_url":"https://api.github.com/repos/sompylasar/Current/assignees{/user}","branches_url":"https://api.github.com/repos/sompylasar/Current/branches{/branch}","tags_url":"https://api.github.com/repos/sompylasar/Current/tags","blobs_url":"https://api.github.com/repos/sompylasar/Current/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/sompylasar/Current/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/sompylasar/Current/git/refs{/sha}","trees_url":"https://api.github.com/repos/sompylasar/Current/git/trees{/sha}","statuses_url":"https://api.github.com/repos/sompylasar/Current/statuses/{sha}","languages_url":"https://api.github.com/repos/sompylasar/Current/languages","stargazers_url":"https://api.github.com/repos/sompylasar/Current/stargazers","contributors_url":"https://api.github.com/repos/sompylasar/Current/contributors","subscribers_url":"https://api.github.com/repos/sompylasar/Current/subscribers","subscription_url":"https://api.github.com/repos/sompylasar/Current/subscription","commits_url":"https://api.github.com/repos/sompylasar/Current/commits{/sha}","git_commits_url":"https://api.github.com/repos/sompylasar/Current/git/commits{/sha}","comments_url":"https://api.github.com/repos/sompylasar/Current/comments{/number}","issue_comment_url":"https://api.github.com/repos/sompylasar/Current/issues/comments{/number}","contents_url":"https://api.github.com/repos/sompylasar/Current/contents/{+path}","compare_url":"https://api.github.com/repos/sompylasar/Current/compare/{base}...{head}","merges_url":"https://api.github.com/repos/sompylasar/Current/merges","archive_url":"https://api.github.com/repos/sompylasar/Current/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/sompylasar/Current/downloads","issues_url":"https://api.github.com/repos/sompylasar/Current/issues{/number}","pulls_url":"https://api.github.com/repos/sompylasar/Current/pulls{/number}","milestones_url":"https://api.github.com/repos/sompylasar/Current/milestones{/number}","notifications_url":"https://api.github.com/repos/sompylasar/Current/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/sompylasar/Current/labels{/name}","releases_url":"https://api.github.com/repos/sompylasar/Current/releases{/id}","deployments_url":"https://api.github.com/repos/sompylasar/Current/deployments","created_at":"2015-01-28T23:57:26Z","updated_at":"2017-05-29T23:52:39Z","pushed_at":"2017-06-15T07:40:58Z","git_url":"git://github.com/sompylasar/Current.git","ssh_url":"git@github.com:sompylasar/Current.git","clone_url":"https://github.com/sompylasar/Current.git","svn_url":"https://github.com/sompylasar/Current","homepage":"https://medium.com/dima-korolev/current-for-realtime-machine-learning-4f04aa8ab81a","size":13242,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"C5T:master","ref":"master","sha":"f05f613eab76f9084373cc3081093cf467a86601","user":{"login":"C5T","id":12224789,"avatar_url":"https://avatars2.githubusercontent.com/u/12224789?v=3","gravatar_id":"","url":"https://api.github.com/users/C5T","html_url":"https://github.com/C5T","followers_url":"https://api.github.com/users/C5T/followers","following_url":"https://api.github.com/users/C5T/following{/other_user}","gists_url":"https://api.github.com/users/C5T/gists{/gist_id}","starred_url":"https://api.github.com/users/C5T/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/C5T/subscriptions","organizations_url":"https://api.github.com/users/C5T/orgs","repos_url":"https://api.github.com/users/C5T/repos","events_url":"https://api.github.com/users/C5T/events{/privacy}","received_events_url":"https://api.github.com/users/C5T/received_events","type":"Organization","site_admin":false},"repo":{"id":27625425,"name":"Current","full_name":"C5T/Current","owner":{"login":"C5T","id":12224789,"avatar_url":"https://avatars2.githubusercontent.com/u/12224789?v=3","gravatar_id":"","url":"https://api.github.com/users/C5T","html_url":"https://github.com/C5T","followers_url":"https://api.github.com/users/C5T/followers","following_url":"https://api.github.com/users/C5T/following{/other_user}","gists_url":"https://api.github.com/users/C5T/gists{/gist_id}","starred_url":"https://api.github.com/users/C5T/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/C5T/subscriptions","organizations_url":"https://api.github.com/users/C5T/orgs","repos_url":"https://api.github.com/users/C5T/repos","events_url":"https://api.github.com/users/C5T/events{/privacy}","received_events_url":"https://api.github.com/users/C5T/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/C5T/Current","description":"C++ framework for realtime machine learning.","fork":false,"url":"https://api.github.com/repos/C5T/Current","forks_url":"https://api.github.com/repos/C5T/Current/forks","keys_url":"https://api.github.com/repos/C5T/Current/keys{/key_id}","collaborators_url":"https://api.github.com/repos/C5T/Current/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/C5T/Current/teams","hooks_url":"https://api.github.com/repos/C5T/Current/hooks","issue_events_url":"https://api.github.com/repos/C5T/Current/issues/events{/number}","events_url":"https://api.github.com/repos/C5T/Current/events","assignees_url":"https://api.github.com/repos/C5T/Current/assignees{/user}","branches_url":"https://api.github.com/repos/C5T/Current/branches{/branch}","tags_url":"https://api.github.com/repos/C5T/Current/tags","blobs_url":"https://api.github.com/repos/C5T/Current/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/C5T/Current/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/C5T/Current/git/refs{/sha}","trees_url":"https://api.github.com/repos/C5T/Current/git/trees{/sha}","statuses_url":"https://api.github.com/repos/C5T/Current/statuses/{sha}","languages_url":"https://api.github.com/repos/C5T/Current/languages","stargazers_url":"https://api.github.com/repos/C5T/Current/stargazers","contributors_url":"https://api.github.com/repos/C5T/Current/contributors","subscribers_url":"https://api.github.com/repos/C5T/Current/subscribers","subscription_url":"https://api.github.com/repos/C5T/Current/subscription","commits_url":"https://api.github.com/repos/C5T/Current/commits{/sha}","git_commits_url":"https://api.github.com/repos/C5T/Current/git/commits{/sha}","comments_url":"https://api.github.com/repos/C5T/Current/comments{/number}","issue_comment_url":"https://api.github.com/repos/C5T/Current/issues/comments{/number}","contents_url":"https://api.github.com/repos/C5T/Current/contents/{+path}","compare_url":"https://api.github.com/repos/C5T/Current/compare/{base}...{head}","merges_url":"https://api.github.com/repos/C5T/Current/merges","archive_url":"https://api.github.com/repos/C5T/Current/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/C5T/Current/downloads","issues_url":"https://api.github.com/repos/C5T/Current/issues{/number}","pulls_url":"https://api.github.com/repos/C5T/Current/pulls{/number}","milestones_url":"https://api.github.com/repos/C5T/Current/milestones{/number}","notifications_url":"https://api.github.com/repos/C5T/Current/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/C5T/Current/labels{/name}","releases_url":"https://api.github.com/repos/C5T/Current/releases{/id}","deployments_url":"https://api.github.com/repos/C5T/Current/deployments","created_at":"2014-12-06T06:52:23Z","updated_at":"2017-05-30T11:39:12Z","pushed_at":"2017-06-15T05:11:11Z","git_url":"git://github.com/C5T/Current.git","ssh_url":"git@github.com:C5T/Current.git","clone_url":"https://github.com/C5T/Current.git","svn_url":"https://github.com/C5T/Current","homepage":"https://medium.com/dima-korolev/current-for-realtime-machine-learning-4f04aa8ab81a","size":13915,"stargazers_count":41,"watchers_count":41,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":14,"mirror_url":null,"open_issues_count":15,"forks":14,"open_issues":15,"watchers":41,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/C5T/Current/pulls/754"},"html":{"href":"https://github.com/C5T/Current/pull/754"},"issue":{"href":"https://api.github.com/repos/C5T/Current/issues/754"},"comments":{"href":"https://api.github.com/repos/C5T/Current/issues/754/comments"},"review_comments":{"href":"https://api.github.com/repos/C5T/Current/pulls/754/comments"},"review_comment":{"href":"https://api.github.com/repos/C5T/Current/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/C5T/Current/pulls/754/commits"},"statuses":{"href":"https://api.github.com/repos/C5T/Current/statuses/eef1ffc0a66d57149693bb76625669aac9579775"}},"merged":false,"mergeable":null,"rebaseable":null,"mergeable_state":"unknown","merged_by":null,"comments":0,"review_comments":0,"maintainer_can_modify":true,"commits":1,"additions":427,"deletions":124,"changed_files":6}}
{"actor":{"display_login":"sompylasar"}}