Split the state correctly for temp cache clearance #1339
Conversation
… delimeter to prevent exhausting cookie length and local storage
|
To summarize: we can take the state string as-is, and remove any local storage values whose key includes the string. Making this logic not dependent on the resource delimiter both solves this bug, but is also future proof if we want to change the format of the state string (which we are considering). |
| this.setItemCookie(key, "", -1); | ||
| this.clearMsalCookie(state); | ||
| } | ||
| if ((!state || key.indexOf(state) !== -1) && !this.tokenRenewalInProgress(state)) { |
There was a problem hiding this comment.
Did we test end-to-end? Will this pass for concurrent silent calls failing (at the msal-core execution) for any reason?
There was a problem hiding this comment.
Yes, I tested with the React sample. Can you clarify the concern with failing concurrent requests?
There was a problem hiding this comment.
This isn't a concern, I was thinking if there will be any cache based on state we do not want to get rid off, that should not be the case.
|
Approving on behalf of @sameerag, who can't approve as the original author. |
This PR fixes #1333 where the
msal-corelib assumes that "state" is always a GUID and does not have a delimiter within itself.We fixed this by making sure that the
guidthemsalattaches before sending a request toserveris always appended at the beginning and we rectify the split logic to reflect this.Note: tests will be added before merging this.