kairi003 / Get-cookies.txt-LOCALLY

Get cookies.txt, NEVER send information outside.
MIT License
320 stars 30 forks source link

Get both partitioned and non-partitioned cookies #61

Closed RainerGevers closed 1 month ago

RainerGevers commented 1 month ago

During testing of the previous fix, I found that sometimes with the new partition key header, some cookies without a partition key was now no longer being retrieved.

My proposed solution is to get the cookies with partition key and cookies without a partition key and merge the results. The new array will contain all cookies with and without partition keys.

In the event of there being cookies with the same name in both sets, the set within the partitioned key section will take precedence.

kairi003 commented 1 month ago

In the event of there being cookies with the same name in both sets, the set within the partitioned key section will take precedence.

I can't find the part of this code that removes duplicates.

However, when I manually created a cookies using Chrome's Dev tool and confirmed its operation, it seemed that multiple cookies with the same name were being sent.

In any case, it is the responsibility of the server to create such a cookie, and I think that there is no problem with the current situation (exporting same name cookies with depending on the presence or absence of a partition-key).

kairi003 commented 1 month ago

Sorry for the inadequate review of the previous version. I overlooked the parts where url is a string and reference url.origin to get undefined, thereby ignoring partition-key.

RainerGevers commented 1 month ago

Sorry for the inadequate review of the previous version. I overlooked the parts where url is a string and reference url.origin to get undefined, thereby ignoring partition-key.

Ah, is that the actual reason for it not working sometimes?

kairi003 commented 1 month ago

First of all, sorry for the delay in thanking you. The idea of merging the cases with and without the pertitionKey is great. It was very helpful that you pointed it out before the release. Thank you very much.

Sorry for the inadequate review of the previous version. I overlooked the parts where url is a string and reference url.origin to get undefined, thereby ignoring partition-key.

Ah, is that the actual reason for it not working sometimes?

Yes, this was occurring at updateBadgeCounter in the background.js and /** Set Cookies data to the table */ in popup.js, where it showed more cookies in the badge and popup than were actually exported.

Also, this is a bug I created, but in updateBadgeCounter, the tabId was always undefined.

I have pushed a commit with these fixes, if you would like to check it out.

Thank you :)

RainerGevers commented 1 month ago

Thanks for the review. I have looked over the changes and tested it with all the exceptions and permutations that I had access to and it seems to work perfectly now. Thanks for the help. LGTM