Skip to content

Commit

Permalink
[8.10] [Security Solution] Fix missing hash in sync to url (#166847) (#…
Browse files Browse the repository at this point in the history
…167010)

# Backport

This will backport the following commits from `main` to `8.10`:
- [[Security Solution] Fix missing hash in sync to url
(#166847)](#166847)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Luke
G","email":"11671118+lgestc@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-09-22T08:23:50Z","message":"[Security
Solution] Fix missing hash in sync to url (#166847)\n\n##
Summary\r\n\r\nThis PR fixes the root cause
for\r\nhttps://github.com//issues/166686
and\r\nhttps://github.com//issues/166774\r\n\r\n@angorayc
@machadoum \r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are
not applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"d52c5a15fdd9e3c8502fbb75378295c5c7649cc6","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Threat
Hunting:Investigations","v8.11.0","v8.10.2"],"number":166847,"url":"#166847
Solution] Fix missing hash in sync to url (#166847)\n\n##
Summary\r\n\r\nThis PR fixes the root cause
for\r\nhttps://github.com//issues/166686
and\r\nhttps://github.com//issues/166774\r\n\r\n@angorayc
@machadoum \r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are
not applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"d52c5a15fdd9e3c8502fbb75378295c5c7649cc6"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"#166847
Solution] Fix missing hash in sync to url (#166847)\n\n##
Summary\r\n\r\nThis PR fixes the root cause
for\r\nhttps://github.com//issues/166686
and\r\nhttps://github.com//issues/166774\r\n\r\n@angorayc
@machadoum \r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are
not applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"d52c5a15fdd9e3c8502fbb75378295c5c7649cc6"}},{"branch":"8.10","label":"v8.10.2","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Luke G <11671118+lgestc@users.noreply.github.com>
  • Loading branch information
kibanamachine and lgestc committed Sep 29, 2023
1 parent 900844b commit 657eaab
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
31 changes: 26 additions & 5 deletions packages/kbn-url-state/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('useSyncToUrl', () => {
window.location = {
...originalLocation,
search: '',
hash: '',
};
window.history = {
...originalHistory,
Expand Down Expand Up @@ -65,9 +66,30 @@ describe('useSyncToUrl', () => {
);
});

it('should should not alter the location hash', () => {
const key = 'testKey';
const valueToSerialize = { test: 'value' };
window.location.hash = '#should_be_there';

const { result } = renderHook(() => useSyncToUrl(key, jest.fn()));

act(() => {
result.current(valueToSerialize);
});

expect(window.history.replaceState).toHaveBeenCalledWith(
{ path: expect.any(String) },
'',
'/#should_be_there?testKey=%28test%3Avalue%29'
);
});

it('should clear the value from the query string on unmount', () => {
const key = 'testKey';

// Location should have a key to clear
window.location.search = `?${key}=${encode({ test: 'value' })}`;

const { unmount } = renderHook(() => useSyncToUrl(key, jest.fn()));

act(() => {
Expand All @@ -85,17 +107,16 @@ describe('useSyncToUrl', () => {
const key = 'testKey';
const restore = jest.fn();

// Location should have a key to clear
window.location.search = `?${key}=${encode({ test: 'value' })}`;

renderHook(() => useSyncToUrl(key, restore, true));

act(() => {
window.dispatchEvent(new Event('popstate'));
});

expect(window.history.replaceState).toHaveBeenCalledTimes(1);
expect(window.history.replaceState).toHaveBeenCalledWith(
{ path: expect.any(String) },
'',
'/?'
);
expect(window.history.replaceState).toHaveBeenCalledWith({ path: expect.any(String) }, '', '/');
});
});
9 changes: 7 additions & 2 deletions packages/kbn-url-state/use_sync_to_url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,15 @@ export const useSyncToUrl = <TValueToSerialize>(
searchParams.delete(key);
}

const newSearch = searchParams.toString();
const stringifiedSearchParams = searchParams.toString();
const newSearch = stringifiedSearchParams.length > 0 ? `?${stringifiedSearchParams}` : '';

if (window.location.search === newSearch) {
return;
}

// Update query string without unnecessary re-render
const newUrl = `${window.location.pathname}?${newSearch}`;
const newUrl = `${window.location.pathname}${window.location.hash}${newSearch}`;
window.history.replaceState({ path: newUrl }, '', newUrl);
},
[key]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,18 @@ type FlyoutState = Parameters<ExpandableFlyoutApi['openFlyout']>[0];
export const useSyncFlyoutStateWithUrl = () => {
const flyoutApi = useRef<ExpandableFlyoutApi>(null);

const syncStateToUrl = useSyncToUrl<FlyoutState>(FLYOUT_URL_PARAM, (data) => {
flyoutApi.current?.openFlyout(data);
});
const handleRestoreFlyout = useCallback(
(state?: FlyoutState) => {
if (!state) {
return;
}

flyoutApi.current?.openFlyout(state);
},
[flyoutApi]
);

const syncStateToUrl = useSyncToUrl<FlyoutState>(FLYOUT_URL_PARAM, handleRestoreFlyout);

// This should be bound to flyout changed and closed events.
// When flyout is closed, url state is cleared
Expand Down

0 comments on commit 657eaab

Please sign in to comment.