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 issue with plugin list download #58

Merged
merged 1 commit into from
Apr 12, 2017
Merged

Conversation

bruderstein
Copy link
Owner

When the plugin list matched the MD5 (which it hasn't done for a while due to a server side issue with the MD5 not being correct!), it would always fail.

The server side issue was corrected, which manifested this issue in the code.

Also, the DEV list MD5 URL wasn't used when the DEV list was selected.

This is the fix for #57

When the plugin list matched the MD5 (which it hasn't done for a while due to a server side issue with the MD5 not being correct!), it would always fail.

The server side issue was corrected, which manifested this issue in the code.

Also, the DEV list MD5 URL wasn't used when the DEV list was selected.
if (downloadResult && serverMD5 != cHashBuffer.get())
if (downloadResult && serverMD5 == cHashBuffer.get()) {
// Server hash matches local hash, so we're ok to continue
downloadSuccess = TRUE;
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is the actual fix. If the hash matched with the server, it would error later.

Copy link
Collaborator

@chcg chcg Apr 11, 2017

Choose a reason for hiding this comment

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

And I already wanna suggest to add a checksum onto the complete list to avoid unnecesary downloads but it seems this was already there.

Could we also find out that the downloaded list is corrupted? Like seen in my post to #57, where it just contained the HTML error page, no xml?

@chcg
Copy link
Collaborator

chcg commented Apr 11, 2017

Wow, just wanna comment on it, but you were too fast. Would have been:

Maybe:

  • a timing related issue not observed so far and now showing up more often.
  • a not initialized value, but that would probably be more sporadic
  • threading synchronization issue

, maybe need to be rechecked with 1.4.3, now the server is up again and see if

How could the issue be explained for the standard list as mentioned in the answer of #57? That one I'm not facing.

Could you also have a look at:

void Plugin::addVersion(const TCHAR* hash, const PluginVersion &version)
{
	tstring *hashString = new tstring;
	*hashString = hash;

	_versionMap[(*hashString)] = version;
}

Mem leak according to cppcheck.

@bruderstein
Copy link
Owner Author

I've changed the server hash and cleared the caches, so it won't be an issue now. There's very aggressive caching on the prod instance, so it wouldn't have kicked in until all the edge caches have the updated hash which matched the contents. It was only when we updated the list to v1.4.5 that the server md5 issue was noticed and fixed, which triggered this issue (just slowly, because of the caching)

@bruderstein
Copy link
Owner Author

Good catch CPPcheck :) I'll push it in a separate PR.

@bruderstein bruderstein merged commit e76bd3c into master Apr 12, 2017
@criskolkman
Copy link

It doesn't seem to be fixed. Just did a fresh install of Notepad++ and added Plugin Manager (shame you have to do it yourself nowadays), but Plugin Manager is not able to get the plugin list.

@chcg chcg deleted the fix-pluginlist-download branch March 4, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants