Skip to content

Commit

Permalink
WEBDEV-6051 Limit length of client_url param and omit it entirely f…
Browse files Browse the repository at this point in the history
…or very long queries (#35)

* Truncate client_url to 400 chars, and omit it if query length exceeds 1000 chars

* Add a missing test for the Metadata class

* Make params wrap properly in demo app
  • Loading branch information
latonv authored May 4, 2023
1 parent cd08ad9 commit 79a1887
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 3 deletions.
8 changes: 6 additions & 2 deletions demo/app-root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,13 @@ export class AppRoot extends LitElement {
${this.lastSearchParams
? html`<div>
Last search params:
<pre>${this.lastSearchParams}</pre>
<pre class="params">${this.lastSearchParams}</pre>
</div>`
: nothing}
${this.lastAggregationParams
? html`<div>
Last aggregation params:
<pre>${this.lastAggregationParams}</pre>
<pre class="params">${this.lastAggregationParams}</pre>
</div>`
: nothing}
</details>
Expand Down Expand Up @@ -613,6 +613,10 @@ export class AppRoot extends LitElement {
align-items: center;
margin-right: 8px;
}
.params {
white-space: pre-wrap;
}
`;
}
}
7 changes: 6 additions & 1 deletion src/search-param-url-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,12 @@ export class SearchParamURLGenerator {
}

if (searchParams.includeClientUrl !== false) {
params.append('client_url', window.location.href);
// Truncate the client_url to 400 characters
const truncatedUrl = window.location.href.slice(0, 400);
// If the query is particularly long, exclude the client_url altogether
if (searchParams.query.length <= 1000) {
params.append('client_url', truncatedUrl);
}
}

return params;
Expand Down
5 changes: 5 additions & 0 deletions src/search-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@ export interface SearchParams {
* * `includeClientUrl: undefined` causes `client_url` param to be included in the request, by default.
* * `includeClientUrl: true` causes `client_url` param to be included, explicitly.
* * `includeClientUrl: false` causes `client_url` param to _not_ be included in the request.
*
* Note that when included, the client URL is truncated to at most 400 characters, to prevent
* overrunning URL length limits in the payload sent to the PPS. Moreover, if the query being
* sent exceeds 1000 characters in length, then the client_url will be omitted from the request
* regardless of this setting.
*/
includeClientUrl?: boolean;
}
73 changes: 73 additions & 0 deletions test/models/metadata.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,84 @@
import { expect } from '@open-wc/testing';

import { Metadata } from '../../src/models/metadata';
import { DateParser } from '@internetarchive/field-parsers';
import { DateField } from '../../src/models/metadata-fields/field-types/date';

// Grab the names of all the getter fields on Metadata objects
// (with the exception of `identifier`, which is just a raw string rather
// than a field-type like the others).
const fieldNames: (keyof Metadata)[] = (Object.keys(
Object.getOwnPropertyDescriptors(Metadata.prototype)
) as (keyof Metadata)[]).filter(
field => !['constructor', 'identifier'].includes(field)
);

describe('Metadata', () => {
it('properly instantiates metadata with identifier', async () => {
const json = { identifier: 'foo', collection: 'bar' };
const metadata = new Metadata(json);
expect(metadata.identifier).to.equal('foo');
expect(metadata.collection?.value).to.equal('bar');
});

it('handles missing data gracefully', () => {
const metadata = new Metadata({});
for (const key of fieldNames) {
expect(metadata[key]).to.be.undefined;
}
});

it('constructs metadata with partial fields', async () => {
const json = {
identifier: 'foo',
title: 'foo-title',
description: 'foo-description',
subject: ['foo-subject1', 'foo-subject2'],
creator: ['foo-creator'],
collection: ['foo-collection'],
date: '2011-07-20T00:00:00Z',
mediatype: 'movies',
item_size: 123456,
files_count: 5,
downloads: 123,
week: 2,
month: 15,
};

const metadata = new Metadata(json);
expect(metadata.rawMetadata).to.deep.equal(json);
expect(metadata.identifier).to.equal(json.identifier);

// Ensure all existing fields are present
for (const key of Object.keys(json)) {
if (key === 'identifier') continue;
const fieldName = key as Exclude<keyof typeof json, 'identifier'>;

if (Array.isArray(json[fieldName])) {
expect(metadata[fieldName]?.values).to.deep.equal(json[fieldName]);
} else if (metadata[fieldName] instanceof DateField) {
expect(metadata[fieldName]?.value).to.deep.equal(
DateParser.shared.parseValue(json[fieldName].toString())
);
} else {
expect(metadata[fieldName]?.value).to.equal(json[fieldName]);
}
}

// Other fields should all be undefined
expect(metadata.addeddate?.value).to.be.undefined;
expect(metadata.avg_rating?.value).to.be.undefined;
expect(metadata.collection_size?.value).to.be.undefined;
expect(metadata.issue?.value).to.be.undefined;
expect(metadata.item_count?.value).to.be.undefined;
expect(metadata.language?.value).to.be.undefined;
expect(metadata.noindex?.value).to.be.undefined;
expect(metadata.num_favorites?.value).to.be.undefined;
expect(metadata.num_reviews?.value).to.be.undefined;
expect(metadata.publicdate?.value).to.be.undefined;
expect(metadata.reviewdate?.value).to.be.undefined;
expect(metadata.source?.value).to.be.undefined;
expect(metadata.type?.value).to.be.undefined;
expect(metadata.volume?.value).to.be.undefined;
});
});

0 comments on commit 79a1887

Please sign in to comment.