Alerting: Fix navigating to URLs with "%" (#96992)

Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com>
pull/97048/head
Gilles De Mey 7 months ago committed by GitHub
parent 2bab11e20a
commit c6d3cf89ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 63
      .yarn/patches/history-npm-4.10.1-ee217563ae.patch
  2. 1
      package.json
  3. 37
      public/app/features/alerting/unified/components/rules/RuleActionsButtons.test.tsx
  4. 0
      public/app/features/alerting/unified/utils/__snapshots__/rule-id.test.tsx.snap
  5. 17
      public/app/features/alerting/unified/utils/rule-id.test.tsx
  6. 14
      yarn.lock

@ -0,0 +1,63 @@
diff --git a/cjs/history.js b/cjs/history.js
index fcd8ebab613c6d87b9ac824feb30ab1080cf0ef2..4df20d5cb2f9ba5fc8777899aada53f49399560b 100644
--- a/cjs/history.js
+++ b/cjs/history.js
@@ -103,16 +103,6 @@ function createLocation(path, state, key, currentLocation) {
if (state !== undefined && location.state === undefined) location.state = state;
}
- try {
- location.pathname = decodeURI(location.pathname);
- } catch (e) {
- if (e instanceof URIError) {
- throw new URIError('Pathname "' + location.pathname + '" could not be decoded. ' + 'This is likely caused by an invalid percent-encoding.');
- } else {
- throw e;
- }
- }
-
if (key) location.key = key;
if (currentLocation) {
diff --git a/esm/history.js b/esm/history.js
index df67820fe3eed558c44fca07a82b0cd409d46720..e0e0d4f69a407e8de782b3fdf8297d42708e110a 100644
--- a/esm/history.js
+++ b/esm/history.js
@@ -80,16 +80,6 @@ function createLocation(path, state, key, currentLocation) {
if (state !== undefined && location.state === undefined) location.state = state;
}
- try {
- location.pathname = decodeURI(location.pathname);
- } catch (e) {
- if (e instanceof URIError) {
- throw new URIError('Pathname "' + location.pathname + '" could not be decoded. ' + 'This is likely caused by an invalid percent-encoding.');
- } else {
- throw e;
- }
- }
-
if (key) location.key = key;
if (currentLocation) {
diff --git a/umd/history.js b/umd/history.js
index 80e4ff66c44a2a71d4f842cc05a252e48dd18e9a..f8f4901be95e48c66f5626fbf051747a2ffbe41d 100644
--- a/umd/history.js
+++ b/umd/history.js
@@ -207,16 +207,6 @@
if (state !== undefined && location.state === undefined) location.state = state;
}
- try {
- location.pathname = decodeURI(location.pathname);
- } catch (e) {
- if (e instanceof URIError) {
- throw new URIError('Pathname "' + location.pathname + '" could not be decoded. ' + 'This is likely caused by an invalid percent-encoding.');
- } else {
- throw e;
- }
- }
-
if (key) location.key = key;
if (currentLocation) {

@ -427,6 +427,7 @@
"debug@npm:^0.7.4": "2.6.9",
"slate-dev-environment@^0.2.2": "patch:slate-dev-environment@npm:0.2.5#.yarn/patches/slate-dev-environment-npm-0.2.5-9aeb7da7b5.patch",
"react-split-pane@0.1.92": "patch:react-split-pane@npm:0.1.92#.yarn/patches/react-split-pane-npm-0.1.92-93dbf51dff.patch",
"history@4.10.1": "patch:history@npm%3A4.10.1#./.yarn/patches/history-npm-4.10.1-ee217563ae.patch",
"redux": "^5.0.0",
"@storybook/blocks@npm:8.1.6": "patch:@storybook/blocks@npm%3A8.1.6#~/.yarn/patches/@storybook-blocks-npm-8.1.6-892f57a6d7.patch",
"react-grid-layout": "patch:react-grid-layout@npm%3A1.4.4#~/.yarn/patches/react-grid-layout-npm-1.4.4-4024c5395b.patch",

@ -1,7 +1,8 @@
import { render, screen, userEvent } from 'test/test-utils';
import { userEvent, screen, render } from 'test/test-utils';
import { byLabelText, byRole } from 'testing-library-selector';
import { config, setPluginLinksHook } from '@grafana/runtime';
import { config, locationService, setPluginLinksHook } from '@grafana/runtime';
import { interceptLinkClicks } from 'app/core/navigation/patch/interceptLinkClicks';
import { contextSrv } from 'app/core/services/context_srv';
import { RuleActionsButtons } from 'app/features/alerting/unified/components/rules/RuleActionsButtons';
import { mockFeatureDiscoveryApi, setupMswServer } from 'app/features/alerting/unified/mockApi';
@ -18,12 +19,16 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto';
import { setupDataSources } from '../../testSetup/datasources';
import { buildInfoResponse } from '../../testSetup/featureDiscovery';
import { fromCombinedRule, stringifyIdentifier } from '../../utils/rule-id';
const server = setupMswServer();
jest.mock('app/core/services/context_srv');
const mockContextSrv = jest.mocked(contextSrv);
// const locationPushSpy = jest.spyOn(locationService, 'push');
const ui = {
detailsButton: byRole('link', { name: /View/ }),
menu: byRole('menu'),
moreButton: byLabelText(/More/),
pauseButton: byRole('menuitem', { name: /Pause evaluation/ }),
@ -109,6 +114,34 @@ describe('RuleActionsButtons', () => {
expect(await getMenuContents()).toMatchSnapshot();
});
it('view rule button should properly handle special characters in rule name', async () => {
// Production setup uses the link interceptor to push all link clicks through the location service
// and history object under the hood
// It causes issues due to the bug in history library that causes the pathname to be decoded
// https://github.com/remix-run/history/issues/505#issuecomment-453175833
document.addEventListener('click', interceptLinkClicks);
grantAllPermissions();
const mockRule = getCloudRule({ name: 'special !@#$%^&*() chars' });
const { user } = render(<RuleActionsButtons rule={mockRule} rulesSource={mimirDs} showViewButton />, {
renderWithRouter: true,
});
const locationPushSpy = jest.spyOn(locationService, 'push');
await user.click(await ui.detailsButton.find());
const ruleId = fromCombinedRule(mimirDs.name, mockRule);
const stringifiedRuleId = stringifyIdentifier(ruleId);
const expectedPath = `/alerting/${encodeURIComponent(mimirDs.name)}/${encodeURIComponent(stringifiedRuleId)}/view`;
// Check if the interceptor worked
expect(locationPushSpy).toHaveBeenCalledWith(expectedPath);
// Check if the location service has the correct pathname
expect(locationService.getLocation().pathname).toBe(expectedPath);
});
it('renders minimal "More" menu when appropriate', async () => {
const user = userEvent.setup();
grantNoPermissions();

@ -1,6 +1,7 @@
import { renderHook } from '@testing-library/react-hooks';
import { TestProvider } from 'test/helpers/TestProvider';
import { config } from '@grafana/runtime';
import { config, locationService } from '@grafana/runtime';
import { AlertingRule, RecordingRule, RuleIdentifier } from 'app/types/unified-alerting';
import {
GrafanaAlertStateDecision,
@ -12,7 +13,7 @@ import {
RulerRecordingRuleDTO,
} from 'app/types/unified-alerting-dto';
import { hashRulerRule, parse, stringifyIdentifier, getRuleIdFromPathname, hashRule, equal } from './rule-id';
import { equal, getRuleIdFromPathname, hashRule, hashRulerRule, parse, stringifyIdentifier } from './rule-id';
const alertingRule = {
prom: {
@ -243,4 +244,16 @@ describe('useRuleIdFromPathname', () => {
expect(result.current).toBe(undefined);
});
it('should decode percent character properly', () => {
locationService.push('/alerting/gdev-cortex/abc%25def/view');
const { result } = renderHook(
() => {
return getRuleIdFromPathname({ id: 'abc%25def' });
},
{ wrapper: TestProvider }
);
expect(result.current).toBe('abc%25def');
});
});

@ -19284,6 +19284,20 @@ __metadata:
languageName: node
linkType: hard
"history@patch:history@npm%3A4.10.1#./.yarn/patches/history-npm-4.10.1-ee217563ae.patch::locator=grafana%40workspace%3A.":
version: 4.10.1
resolution: "history@patch:history@npm%3A4.10.1#./.yarn/patches/history-npm-4.10.1-ee217563ae.patch::version=4.10.1&hash=b8466f&locator=grafana%40workspace%3A."
dependencies:
"@babel/runtime": "npm:^7.1.2"
loose-envify: "npm:^1.2.0"
resolve-pathname: "npm:^3.0.0"
tiny-invariant: "npm:^1.0.2"
tiny-warning: "npm:^1.0.0"
value-equal: "npm:^1.0.1"
checksum: 10/15ef2925dc665d7671308625a03e43b0c2e5b7d7a28c41b04c3ca68c805b6c4b6c6e0c217168ebf356c753ccdce6236ad717b58e6dae92be0501a0bfd44c0423
languageName: node
linkType: hard
"hoist-non-react-statics@npm:3.3.2, hoist-non-react-statics@npm:^3.1.0, hoist-non-react-statics@npm:^3.3.0, hoist-non-react-statics@npm:^3.3.1, hoist-non-react-statics@npm:^3.3.2":
version: 3.3.2
resolution: "hoist-non-react-statics@npm:3.3.2"

Loading…
Cancel
Save