Skip to content
This repository was archived by the owner on Oct 29, 2025. It is now read-only.

Added cookie name extraction#66

Merged
yaronschwimmer merged 6 commits into
devfrom
dev-extract-cookie-names
Nov 1, 2018
Merged

Added cookie name extraction#66
yaronschwimmer merged 6 commits into
devfrom
dev-extract-cookie-names

Conversation

@alexbpx

@alexbpx alexbpx commented Oct 31, 2018

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread lib/pxutil.js Outdated
function extractCookieNames(cookieHeader) {
var result;
if(cookieHeader){
var cookies = cookieHeader.split(';');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer let/const over var

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread lib/pxutil.js Outdated
if(cookieHeader){
var cookies = cookieHeader.split(';');
var cookieNames = new Array (cookies.length);
for (var i = 0; i < cookies.length ; i++){

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a single javascript line can do all this at once:

Suggested change
for (var i = 0; i < cookies.length ; i++){
const cookieNames = cookies.map(cookie => cookie.split('=')[0].trim());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread lib/pxapi.js Outdated
module_version: config.MODULE_VERSION,
cookie_origin: pxCtx.cookieOrigin
cookie_origin: pxCtx.cookieOrigin,
risk_cookie_names: pxCtx.risk_cookie_names

@yaronschwimmer yaronschwimmer Nov 1, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be pxCtx.riskCookieNames

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how did work when you tested it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked because I tested in with the correct name. Don't knwo how the name switch has happened

Comment thread test/pxutils.test.js
it('should extract cookie names from the cookie header', (done) => {
var cookieHeader = '_px3=px3Cookie;tempCookie=CookieTemp; _px7=NotARealCookie';
var formattedHeaders = pxutil.extractCookieNames(cookieHeader);
(Object.prototype.toString.call(formattedHeaders)).should.be.exactly('[object Array]');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not formattedHeaders.toString()? this will actually pretty print the array instead of the general [object Array]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I want to know the type, not the value

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case

Suggested change
(Object.prototype.toString.call(formattedHeaders)).should.be.exactly('[object Array]');
Array.isArray(formattedHeaders).should.be.true()

@yaronschwimmer yaronschwimmer merged commit ff1fd22 into dev Nov 1, 2018
@yaronschwimmer yaronschwimmer deleted the dev-extract-cookie-names branch November 1, 2018 15:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants