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.

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

fix(HTTP Request Node): Handle special characters in pagination expressions + improve hint text #8576

Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,10 @@ export class ChatTrigger implements INodeType {
await validateAuth(this);
} catch (error) {
if (error) {
res.writeHead(error.responseCode as number, {
res.writeHead((error as IDataObject).responseCode as number, {
'www-authenticate': 'Basic realm="Webhook"',
});
res.end(error.message as string);
res.end((error as IDataObject).message as string);
return { noWebhookResponse: true };
}
throw error;
Expand Down
24 changes: 23 additions & 1 deletion packages/core/src/NodeExecuteFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,17 @@ const createFormDataObject = (data: Record<string, unknown>) => {
return formData;
};

const validateUrl = (url?: string): boolean => {
if (!url) return false;

try {
new URL(url);
return true;
} catch (error) {
return false;
}
};

function searchForHeader(config: AxiosRequestConfig, headerName: string) {
if (config.headers === undefined) {
return undefined;
Expand Down Expand Up @@ -1240,7 +1251,10 @@ function applyPaginationRequestData(
requestData: OptionsWithUri,
paginationRequestData: PaginationOptions['request'],
): OptionsWithUri {
const preparedPaginationData: Partial<OptionsWithUri> = { ...paginationRequestData };
const preparedPaginationData: Partial<OptionsWithUri> = {
...paginationRequestData,
uri: paginationRequestData.url,
};

if ('formData' in requestData) {
preparedPaginationData.formData = paginationRequestData.body;
Expand Down Expand Up @@ -2885,6 +2899,14 @@ const getRequestHelperFunctions = (

const tempRequestOptions = applyPaginationRequestData(requestOptions, paginateRequestData);

if (!validateUrl(tempRequestOptions.uri as string)) {
throw new NodeOperationError(node, `'${paginateRequestData.url}' is not a valid URL.`, {
itemIndex,
runIndex,
type: 'invalid_url',
Copy link
Contributor

Choose a reason for hiding this comment

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

I would avoid extending NodeApiError interface for this special case, as this error would be replaced in Http node anyway we could send some technical message for identification

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer using a type to check on instead of using the message. We also use this pattern for other errors (like ExpressionError)

});
}

if (credentialsType) {
tempResponseData = await this.helpers.requestWithAuthentication.call(
this,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,21 @@
</template>

<script lang="ts">
import { defineComponent } from 'vue';
import type { PropType } from 'vue';
import { mapStores } from 'pinia';
import { EditorView, keymap } from '@codemirror/view';
import { Compartment, EditorState, Prec } from '@codemirror/state';
import { history, redo, undo } from '@codemirror/commands';
import { acceptCompletion, autocompletion, completionStatus } from '@codemirror/autocomplete';
import { history, redo, undo } from '@codemirror/commands';
import { Compartment, EditorState, Prec } from '@codemirror/state';
import { EditorView, keymap } from '@codemirror/view';
import type { PropType } from 'vue';
import { defineComponent } from 'vue';

import { useNDVStore } from '@/stores/ndv.store';
import { completionManager } from '@/mixins/completionManager';
import { expressionManager } from '@/mixins/expressionManager';
import { highlighter } from '@/plugins/codemirror/resolvableHighlighter';
import { expressionInputHandler } from '@/plugins/codemirror/inputHandlers/expression.inputHandler';
import { inputTheme } from './theme';
import { n8nLang } from '@/plugins/codemirror/n8nLang';
import { completionManager } from '@/mixins/completionManager';
import { highlighter } from '@/plugins/codemirror/resolvableHighlighter';
import { isEqual } from 'lodash-es';
import type { IDataObject } from 'n8n-workflow';
import { inputTheme } from './theme';

const editableConf = new Compartment();

Expand Down Expand Up @@ -68,26 +67,18 @@ export default defineComponent({
},
});
},
ndvInputData() {
this.editor?.dispatch({
changes: {
from: 0,
to: this.editor.state.doc.length,
insert: this.modelValue,
},
});
displayableSegments(segments, newSegments) {
if (isEqual(segments, newSegments)) return;

setTimeout(() => {
this.editor?.contentDOM.blur();
highlighter.removeColor(this.editor, this.plaintextSegments);
highlighter.addColor(this.editor, this.resolvableSegments);

this.$emit('change', {
value: this.unresolvedExpression,
segments: this.displayableSegments,
});
},
},
computed: {
...mapStores(useNDVStore),
ndvInputData(): object {
return this.ndvStore.ndvInputData;
},
},
mounted() {
const extensions = [
n8nLang(),
Expand Down Expand Up @@ -125,19 +116,11 @@ export default defineComponent({
// Force segments value update by keeping track of editor state
this.editorState = this.editor.state;

highlighter.removeColor(this.editor, this.plaintextSegments);
highlighter.addColor(this.editor, this.resolvableSegments);

setTimeout(() => {
try {
this.trackCompletion(viewUpdate, this.path);
} catch {}
});

this.$emit('change', {
value: this.unresolvedExpression,
segments: this.displayableSegments,
});
}),
];

Expand All @@ -148,14 +131,10 @@ export default defineComponent({
extensions,
}),
});

this.editorState = this.editor.state;

highlighter.addColor(this.editor, this.resolvableSegments);

this.$emit('change', {
value: this.unresolvedExpression,
segments: this.displayableSegments,
});
},
beforeUnmount() {
this.editor?.destroy();
Expand Down
3 changes: 2 additions & 1 deletion packages/editor-ui/src/components/Node.vue
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,8 @@ export default defineComponent({
this.$emit('removeNode', this.data.name);
},

toggleDisableNode() {
toggleDisableNode(event: MouseEvent) {
(event.currentTarget as HTMLButtonElement).blur();
this.$telemetry.track('User clicked node hover button', {
node_type: this.data.type,
button_name: 'disable',
Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/composables/useWorkflowHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export function resolveParameter(
// in pagination expressions
additionalKeys.$response = get(
executionData,
`data.executionData.contextData['node:${activeNode.name}'].response`,
['data', 'executionData', 'contextData', `node:${activeNode.name}`, 'response'],
{},
);
}
Expand Down
30 changes: 21 additions & 9 deletions packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ export class HttpRequestV3 implements INodeType {
},
default: '',
description:
'Should evaluate to true when pagination is complete. More info.',
'Should evaluate to the URL of the next page. <a href="https://docs.n8n.io/integrations/builtin/core-nodes/n8n-nodes-base.httprequest/#pagination" target="_blank">More info</a>.',
},
{
displayName: 'Parameters',
Expand Down Expand Up @@ -1112,7 +1112,7 @@ export class HttpRequestV3 implements INodeType {
},
default: '',
description:
'Should evaluate to true when pagination is complete. More info.',
'Should evaluate to true when pagination is complete. <a href="https://docs.n8n.io/integrations/builtin/core-nodes/n8n-nodes-base.httprequest/#pagination" target="_blank">More info</a>.',
},
{
displayName: 'Limit Pages Fetched',
Expand Down Expand Up @@ -1704,13 +1704,25 @@ export class HttpRequestV3 implements INodeType {
paginationData.binaryResult = true;
}

const requestPromise = this.helpers.requestWithAuthenticationPaginated.call(
this,
requestOptions,
itemIndex,
paginationData,
nodeCredentialType ?? genericCredentialType,
);
const requestPromise = this.helpers.requestWithAuthenticationPaginated
.call(
this,
requestOptions,
itemIndex,
paginationData,
nodeCredentialType ?? genericCredentialType,
)
.catch((error) => {
if (error instanceof NodeOperationError && error.type === 'invalid_url') {
const urlParameterName =
pagination.paginationMode === 'responseContainsNextURL' ? 'Next URL' : 'URL';
throw new NodeOperationError(this.getNode(), error.message, {
description: `Make sure the "${urlParameterName}" parameter evaluates to a valid URL.`,
});
}

throw error;
});
requestPromises.push(requestPromise);
} else if (authentication === 'genericCredentialType' || authentication === 'none') {
if (oAuth1Api) {
Expand Down
Loading
Loading