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

Downloads fail if server doesn't set Connection: keep-alive #147

Closed
sinall opened this issue Sep 5, 2017 · 12 comments
Closed

Downloads fail if server doesn't set Connection: keep-alive #147

sinall opened this issue Sep 5, 2017 · 12 comments

Comments

@sinall
Copy link

sinall commented Sep 5, 2017

I debugged into the source code, I think it was introduced in commit:
Don't fail downloads on redirects to another server

In my case, it can easily go to line 355 !!

This happens on version > 0.5.2.

@vslavik
Copy link
Owner

vslavik commented Sep 5, 2017

Please read this and provide clear description and reproduction instructions.

@sinall
Copy link
Author

sinall commented Sep 5, 2017

I use 0.5.2 previously, works fine, I tried to upgrade it to 0.5.6 (or 0.5.3), both show me the same error as I mentioned.

I don't know what else I could provide. What I can tell is it throws exception when downloading feed file.

@vslavik
Copy link
Owner

vslavik commented Sep 5, 2017

I don't know what else I could provide

Which is why I asked you to please read that article. It explains things.

In short: ask yourself if a developer incapable of reading your mind and having no access to your computer reproduce the issue? If not, your bug report is lacking necessary information.

Give me some credit: do you think I'm totally incompetent? Don't you think that maybe, just maybe everything does in fact work correctly for me and, it would seem, everybody else using WinSparkle? That perhaps you're being a little bit too vague and over-assuming and there is in fact some issue specific to whatever secret thing you do? To your data, feeds, OS, settings, code, etc.?

@sinall
Copy link
Author

sinall commented Sep 5, 2017

OS: Win10 64bit
VC++ 2015, build with 'Use Unicode Character Set'
Winsparkle version: 0.5.3 and 0.5.6
Some pieces of code:

Setup:

	std::wstring appcastUrl = std::wstring(Constants::SERVER_BASE_URL) + L"/download/e/version-info.xml";
	win_sparkle_set_appcast_url(StringConversionUtils::ConvertWStrToUtf8Str(appcastUrl).c_str());
	win_sparkle_set_automatic_check_for_updates(1);
	win_sparkle_set_lang("zh-CN");
	win_sparkle_init();
	win_sparkle_set_shutdown_request_callback(&OnSparkleShutdown);

Then, when menu is clicked:
win_sparkle_check_update_with_ui();

Again, after I switch back to 0.5.2, it works again.

@vslavik
Copy link
Owner

vslavik commented Sep 5, 2017

Once again, for the last time: Please do read the damn article. Afterwards, please provide reproducible instructions (hint: code snippets that refer undefined symbols are not: I can't read your mind and fetch the values from it). This is not complicated. If you're having problems with your server, nobody is going to be able to help you if you continue to be secretive and without essential part of the code, such as the actual feed URL.

In fact, let me make it fool-proof: please provide a complete and working patch against the example included with WinSparkle. If you can reproduce it there, it rules out mistakes on your end — and provides enough information for others to replicate your findings.

@sinall
Copy link
Author

sinall commented Sep 5, 2017

MFCApplication2.zip

Copy MFCApplication2\WinSparkle-0.5.6\Release\WinSparkle.dll to MFCApplication2\Debug before run it.

Very simple code, only 4 lines related with Winsparkle in MFCApplication2Dlg.cpp:

	win_sparkle_set_appcast_url("http://115.29.177.153/download/test/appcast.xml");
	win_sparkle_set_automatic_check_for_updates(1);
	win_sparkle_init();
	win_sparkle_check_update_with_ui();

This could reproduce the problem easily.

@vslavik
Copy link
Owner

vslavik commented Sep 5, 2017

I'm sorry, is it really necessary to repeat everything at least twice before you stop ignoring it?

please provide a complete and working patch against the example included with WinSparkle.

@sinall
Copy link
Author

sinall commented Sep 5, 2017

Just a bug report, don't have a patch yet.

@vslavik
Copy link
Owner

vslavik commented Sep 5, 2017

A patch to apply to the example to demonstrate the issue. See above. Not some separate application that has to be compile who knows how with who knows what other problems. The simplest possible thing: a diff showing modifications to the existing example sufficient to reproduce the issue. This is standard practice in bug reporting, so please do it.

@sinall
Copy link
Author

sinall commented Sep 5, 2017

Sorry, I might not be able to give you a patch, I tried to build the project, it has 173 errors.
I think the above example could reproduce the issue easily.

In the above example I attached, it fails with url:

http://115.29.177.153/download/test/appcast.xml

But successes with url:

https://winsparkle.org/example/appcast.xml

@vslavik
Copy link
Owner

vslavik commented Sep 5, 2017

I tried to build the project, it has 173 errors.

There's a file called "README" in there. Guess what it's called that :(

@vslavik
Copy link
Owner

vslavik commented Sep 6, 2017

The principal difference appears to be that your server doesn't send Connection: keep-alive.

That would explain the relationship to b9f988a and means that the #114 PR that introduced both problems is inherently wrong :(

@vslavik vslavik changed the title "are you connected to the internet" in 0.5.6 Downloads fail if server doesn't set Connection: keep-alive Sep 9, 2017
@vslavik vslavik closed this as completed in 78f8654 Sep 9, 2017
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

No branches or pull requests

2 participants