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

reworked a way of request url parts parsing #111

Merged
merged 5 commits into from
Apr 14, 2024
Merged

reworked a way of request url parts parsing #111

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4ce71ae
reworked a way of request url parts parsing
romansertsov Apr 12, 2024
0f2d5db
Added test coverage for addFromRequest function of Evidence class
romansertsov Apr 14, 2024
cbadfec
minor addFromRequest refactor
romansertsov Apr 14, 2024
8d51f43
added full url from request test
romansertsov Apr 14, 2024
27fa5a3
linting errors cleanup
romansertsov Apr 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions fiftyone.pipeline.core/evidence.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,23 @@ class Evidence {

evidence.add('server.host-ip', request.connection.localAddress.toString());

// Get querystring data
// Parsing URL using new URL constructor

const protocol = request.protocol || 'http';
const hostname = request.hostname || 'localhost'; // Use a default hostname if not available
const port = request.socket.localPort || ''; // Port may not be available in some cases
const searchParams = request.query || ''; // Assuming query parameters are in req.query
Copy link
Contributor

Choose a reason for hiding this comment

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

Is request an IncomingMessage type? If so it will have the .url property that can then be parsed... and then searchParams can be taken from url.URL(request.url).searchParams according to the docs.. so the previous implementation seems to have been correct...


let fullURL = `${protocol}://${hostname}${port ? `:${port}` : ''}${request.url}`;

const params = new url.URL(request.url);
if (searchParams) {
fullURL += `?${searchParams}`;
}

// Get querystring data

const query = params.searchParams;
const URL = new url.URL(fullURL);
const query = URL.searchParams;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have some tests directly in this package that would check that the evidence is correctly parsed from query params?


Object.keys(query).forEach(function (key) {
const value = query[key];
Expand Down