Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RNTester/js/http_test_server.js Uses express API with connect middleware. #17899

Closed
ej3 opened this issue Feb 7, 2018 · 1 comment
Closed

RNTester/js/http_test_server.js Uses express API with connect middleware. #17899

ej3 opened this issue Feb 7, 2018 · 1 comment
Labels
Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.

Comments

@ej3
Copy link
Contributor

ej3 commented Feb 7, 2018

Is this a bug report?

Yes

Steps to Reproduce

  1. git clone [email protected]:facebook/react-naive.git
  2. cd react-native
  3. npm install
  4. ./RNTester/js/http_test_server.js &
  5. curl -D - localhost:5556

Expected Behavior

Server Output

Test server for WebSocketExample

This will set a cookie named "wstest" on the response of any incoming request.

received request

Server Response

HTTP/1.1 200 OK
X-Powered-By: Express
Set-Cookie: wstest=OK; Path=/
Date: Wed, 07 Feb 2018 21:36:13 GMT
Connection: keep-alive
Content-Length: 21

Cookie has been set!

Actual Behavior

Server Output

Test server for WebSocketExample

This will set a cookie named "wstest" on the response of any incoming request.

received request
TypeError: res.cookie is not a function
    at /mnt/c/Users/ejbru/dev/appittome/react-native/RNTester/js/http_test_server.js:37:7
    at call (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:239:7)
    at next (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:183:5)
    at Function.handle (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:186:3)
    at Server.app (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:51:37)
    at emitTwo (events.js:106:13)
    at Server.emit (events.js:194:7)
    at parserOnIncoming (_http_server.js:565:12)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:99:23)

Server Response

HTTP/1.1 500 Internal Server Error
Content-Security-Policy: default-src 'self'
X-Content-Type-Options: nosniff
Content-Type: text/html; charset=utf-8
Content-Length: 960
Date: Wed, 07 Feb 2018 21:22:09 GMT
Connection: keep-alive

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>TypeError: res.cookie is not a function<br> &nbsp; &nbsp;at /mnt/c/Users/ejbru/dev/appittome/react-native/RNTester/js/http_test_server.js:37:7<br> &nbsp; &nbsp;at call (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:239:7)<br> &nbsp; &nbsp;at next (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:183:5)<br> &nbsp; &nbsp;at Function.handle (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:186:3)<br> &nbsp; &nbsp;at Server.app (/mnt/c/Users/ejbru/dev/appittome/react-native/node_modules/connect/index.js:51:37)<br> &nbsp; &nbsp;at emitTwo (events.js:106:13)<br> &nbsp; &nbsp;at Server.emit (events.js:194:7)<br> &nbsp; &nbsp;at parserOnIncoming (_http_server.js:565:12)<br> &nbsp; &nbsp;at HTTPParser.parserOnHeadersComplete (_http_common.js:99:23)</pre>
</body>
</html>

What's happening

@antoinerousseau
Commit be4afdd creates a service using connect middleware

const connect = require('connect');
const http = require('http');

const app = connect();

then proceeds to use the app with an express/middleware API instead of the vanilla connect API.

app.use(function(req, res) {
  console.log('received request');
  const cookieOptions = {
    //httpOnly: true, // the cookie is not accessible by the user (javascript,...)
    secure: false, // allow HTTP
  };
  res.cookie('wstest', 'OK', cookieOptions);
  res.end('Cookie has been set!\n');
});

What to do?

I feel like the only reason I ran into this problem is because no one has actually ever used this server to test websockets. So we could

  1. Remove it
  2. Just use res.setHeader()
  3. Add some middleware to connect (new project dependency)
  4. Just use expressjs (new project dependency)

I'll post a PR with option 2 that can just be merged unless someone wants option 1.
@ej3 ej3 mentioned this issue Feb 7, 2018
Closed
facebook-github-bot pushed a commit that referenced this issue Feb 13, 2018
@ej3 @facebook-github-bot
RNTester http_server send cookie fix
03e6d3e 
Summary:
Signed-off-by: Evan J Brunner <[email protected]>

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Motivation can be found in #17899

This `RNTester/js/http_test_server.js` is part of a internal websocket test suite / devtool.

Can be tested with `curl -D - localhost:5556` observing that the `Set-Cookie: wstest=OK; Path=\` header is present, and the service throws no exceptions.. etc

[INTERNAL][MINOR][./RNTester/js/http_test_server.js] - fixed set cookie with connect framework
<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes #17900

Differential Revision: D6977087

Pulled By: hramos

fbshipit-source-id: af6205343fccf69c57e0c26a85a5b04d61288a23
Plo4ox pushed a commit to Plo4ox/react-native that referenced this issue Feb 17, 2018
@ej3
RNTester http_server send cookie fix
145f63a 
Summary:
Signed-off-by: Evan J Brunner <[email protected]>

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Motivation can be found in facebook#17899

This `RNTester/js/http_test_server.js` is part of a internal websocket test suite / devtool.

Can be tested with `curl -D - localhost:5556` observing that the `Set-Cookie: wstest=OK; Path=\` header is present, and the service throws no exceptions.. etc

[INTERNAL][MINOR][./RNTester/js/http_test_server.js] - fixed set cookie with connect framework
<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#17900

Differential Revision: D6977087

Pulled By: hramos

fbshipit-source-id: af6205343fccf69c57e0c26a85a5b04d61288a23
@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like you may not be using the latest version of React Native, v0.53.0, released on January 2018. Can you make sure this issue can still be reproduced in the latest version?

I am going to close this, but please feel free to open a new issue if you are able to confirm that this is still a problem in v0.53.0 or newer.

How to Contribute • What to Expect from Maintainers

@react-native-bot react-native-bot added Ran Commands One of our bots successfully processed a command. Stale There has been a lack of activity on this issue and it may be closed soon. labels Feb 24, 2018
@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 24, 2018
@facebook facebook locked as resolved and limited conversation to collaborators Feb 24, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ej3 @react-native-bot