Skip to content

Commit

Permalink
fix: Fixed bug with empty Accept headers and internal/quads bodies
Browse files Browse the repository at this point in the history
In case the Accept header was empty and the store returned an internal/quads
resource, our  mechanism to prevent internal data from being returned
was ignored.
  • Loading branch information
joachimvh committed Feb 9, 2021
1 parent 6c4378a commit 59600b0
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
5 changes: 4 additions & 1 deletion src/storage/conversion/ConversionUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpErr
* Since more specific media ranges override less specific ones,
* this will be ignored if there is a specific internal type preference.
*
* This should be called even if there are no preferredTypes since this also filters out internal types.
*
* @param preferredTypes - Preferences for output type.
* @param availableTypes - Media types to compare to the preferences.
*
Expand All @@ -24,8 +26,9 @@ string[] {
const preferred = { ...preferredTypes };
if (Object.keys(preferredTypes).length === 0) {
preferred['*/*'] = 1;
}
// Prevent accidental use of internal types
} else if (!(INTERNAL_ALL in preferred)) {
if (!(INTERNAL_ALL in preferred)) {
preferred[INTERNAL_ALL] = 0;
}

Expand Down
10 changes: 3 additions & 7 deletions src/storage/conversion/IfNeededConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ export class IfNeededConverter extends RepresentationConverter {
}

protected needsConversion({ identifier, representation, preferences }: RepresentationConverterArgs): boolean {
// No conversion needed if no preferences were specified
if (!preferences.type) {
return false;
}

// No conversion is needed if there are any matches for the provided content type
const { contentType } = representation.metadata;
if (!contentType) {
Expand All @@ -49,8 +44,9 @@ export class IfNeededConverter extends RepresentationConverter {
const noMatchingMediaType = !hasMatchingMediaTypes(preferences.type, { [contentType]: 1 });
if (noMatchingMediaType) {
this.logger.debug(`Conversion needed for ${identifier
.path} from ${representation.metadata.contentType} to satisfy ${Object.entries(preferences.type)
.map(([ value, weight ]): string => `${value};q=${weight}`).join(', ')}`);
.path} from ${representation.metadata.contentType} to satisfy ${!preferences.type ?
'""' :
Object.entries(preferences.type).map(([ value, weight ]): string => `${value};q=${weight}`).join(', ')}`);
}
return noMatchingMediaType;
}
Expand Down
14 changes: 13 additions & 1 deletion test/unit/storage/conversion/ConversionUtil.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import type { ValuePreferences } from '../../../../src/ldp/representation/RepresentationPreferences';
import {
hasMatchingMediaTypes,
matchesMediaType,
matchingMediaTypes,
supportsMediaTypeConversion,
} from '../../../../src/storage/conversion/ConversionUtil';
import { InternalServerError } from '../../../../src/util/errors/InternalServerError';

describe('ConversionUtil', (): void => {
describe('supportsMediaTypeConversion', (): void => {
describe('#supportsMediaTypeConversion', (): void => {
it('requires preferences.', async(): Promise<void> => {
expect((): any => supportsMediaTypeConversion()).toThrow();
});
Expand Down Expand Up @@ -82,6 +83,17 @@ describe('ConversionUtil', (): void => {
});
});

describe('#hasMatchingMediatypes', (): void => {
it('returns false if there are no matches.', async(): Promise<void> => {
expect(hasMatchingMediaTypes()).toEqual(false);
});

it('returns true if there are matches.', async(): Promise<void> => {
const preferences: ValuePreferences = { 'a/x': 1, 'b/x': 0.5, 'c/x': 0 };
expect(hasMatchingMediaTypes(preferences, { 'b/x': 1, 'c/x': 1 })).toEqual(true);
});
});

describe('#matchesMediaType', (): void => {
it('matches all possible media types.', async(): Promise<void> => {
expect(matchesMediaType('*/*', 'text/turtle')).toBeTruthy();
Expand Down
15 changes: 15 additions & 0 deletions test/unit/storage/conversion/IfNeededConverter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ describe('An IfNeededConverter', (): void => {
expect(innerConverter.handleSafe).toHaveBeenCalledTimes(0);
});

it('performs conversion when there are no preferences but the content-type is internal.', async(): Promise<void> => {
const preferences = {};
const internalRepresentation = {
metadata: { contentType: 'internal/quads' },
} as any;
const args = { identifier, representation: internalRepresentation, preferences };

await expect(converter.handleSafe(args)).resolves.toBe(converted);

expect(innerConverter.canHandle).toHaveBeenCalledTimes(0);
expect(innerConverter.handle).toHaveBeenCalledTimes(0);
expect(innerConverter.handleSafe).toHaveBeenCalledTimes(1);
expect(innerConverter.handleSafe).toHaveBeenCalledWith(args);
});

it('errors if no content type is specified on the representation.', async(): Promise<void> => {
const preferences = { type: { 'text/turtle': 1 }};
const args = { identifier, representation: { metadata: {}} as any, preferences };
Expand Down

0 comments on commit 59600b0

Please sign in to comment.