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

HGET: Version 1.3 w/ TCP-IP 1.1 support and HTTPS support #10

Merged
merged 8 commits into from Aug 14, 2019

Conversation

@ducasp
Copy link
Contributor

commented Jul 25, 2019

  • Check if TLS is supported by UNAPI implementation
  • Check if UNAPI implementation supports safe TLS (validating host certificate)
  • If protocol is HTTPS and UNAPI implementation supports TLS, open connection using TLS
  • Added /u to allow UNSAFE https transfers if adapter supports safe TLS connections
  • Added /n to allow to not check hostname if adapter supports safe TLS connections
  • There is no need for waiting for an extra tick after calling TCPIP_WAIT. Implementations will make sure of that, if they need it.
  • Changed the progress indication, now a progress bar that is increased in 4% increments. This is easier on CPU/VDP waits and allow faster adapters to get more data instead of waiting for screen to be updated. If file size is unknown, a star-like movement is show to indicate program is not stuck.
  • When connection closed reason is available, show it always, even for implementations that do not block on TCP_OPEN.
ducasp added 5 commits Jul 19, 2019
Version 1.3 w/ TCP-IP 1.1 support and HTTPS support
- Check if TLS is supported by UNAPI implementation
- Check if UNAPI implementation supports safe TLS (validating host certificate)
- If protocol is HTTPS and UNAPI implementation supports TLS, open connection using TLS
- Added /u to allow UNSAFE https transfers if adapter supports safe TLS connections
- Added /n to allow to not check hostname if adapter supports safe TLS connections
- There is no need for waiting for an extra tick after calling TCPIP_WAIT. Implementations will make sure of that, if they need it. 
- Changed the progress indication, now a progress bar that is increased in 4% increments. This is easier on CPU/VDP waits and allow faster adapters to get more data instead of waiting for screen to be updated. If file size is unknown, a star-like movement is show to indicate program is not stuck.
v1.3 Changes
Added suggested changes by Nestor to make code easier to understand and follow with his way of declaring variables.
Removed inline ASM function as it can be replaced by printing a few to the left (29 or 0x1d) characters
Minor fix to progress bar
Progress bar would miss a few blocks in some occasions.
Better handling of connection failure
Now have a list of reasons that can be used both for ERR_NO_CONN on TCP_OPEN as well as TCP_STATE, and give the connection failure reason for both cases of adapters that return ERR_NO_CONN on TCP_OPEN (blocking TCP_OPEN) as well on TCP_STATE (non-blocking TCP_OPEN)
Better explanation of connection failure
Allow showing the connection failure reason not only for https connection but also http when the adapter blocks on TCP_OPEN

@Konamiman Konamiman changed the title Ducasp hget1.3 patch1 HGET: Version 1.3 w/ TCP-IP 1.1 support and HTTPS support Jul 26, 2019

@Konamiman

This comment has been minimized.

Copy link
Owner

commented Jul 26, 2019

There was no need to open a separate pull request for this, given that the original one was still open. You should have added the new commits to the existing pull request.

However, since the original pull request was based on master and I see that this new one still contains all the commits, I'm closing the old one and keeping this one. Please work on this branch and pull request for further changes.

@Konamiman

This comment has been minimized.

Copy link
Owner

commented Jul 26, 2019

There's a missing scenario here: redirection from HTTP to HTTPS.

Consider this: hget www.konamiman.com. This will connect to port 80 using regular HTTP. The server will reply with a 3xx status (redirect request) and a Location: https://www.konamiman.com/ header. The program should detect the https:// part of the new URL and do the new request using TLS (using either the default HTTPS port, or the port supplied in the URL). Instead, it's using HTTP again, which causes an infinite loop of redirections.

Of course, if the TCP/IP implementation doesn't support TLS, then such a redirection request should cause a "unsupported protocol" error, just as v1.0 of the program does.

Additionally, if I remember correctly the ofiginal program had a mechanism to prevent infinite redirection loops (it stopped after a few redirections), which doesn't seem to be working in this case, could you please take a look?

@Konamiman

This comment has been minimized.

Copy link
Owner

commented Jul 26, 2019

On a positive note, the new progress indicator looks great! 👍

@ducasp

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2019

There was no need to open a separate pull request for this, given that the original one was still open. You should have added the new commits to the existing pull request.

However, since the original pull request was based on master and I see that this new one still contains all the commits, I'm closing the old one and keeping this one. Please work on this branch and pull request for further changes.

Yes, that was the reason why I've chosen to make a new commit, to not worry about master again. :-)

@ducasp

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2019

Hi Nestor,

Did not find any reference to a mechanism to prevent infinite redirection loops on the code available, I've used a simple approach like the one in MSX HUB, count redirections and if 10 is hit, stops.

Also have changed the code so http -> https and https -> http redirections should work fine.

@Konamiman Konamiman force-pushed the ducasp:ducasp-hget1.3-patch1 branch from 09facc8 to 7e86f42 Jul 31, 2019

@Konamiman

This comment has been minimized.

Copy link
Owner

commented Jul 31, 2019

Weird, I remember having implemented a redirection limit. Must have been in another tool. 🤷🏻‍♂️

All looks good now. I have pushed a minor change: now hget without parameters displays short help, and hget ? displays extended help. Run git pull in your console to get the new commit and test it if you want.

Allow me a couple of weeks to go back home and test this (and a new version of InterNestor I'm working on) in my real MSX, then I'll accept this pull request and the other networking related ones.

@ducasp

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

I've tested it and it is working great here. :) 👍

Last minute imrpovements:
- Check if TLS is supported only of the TCP/IP specification version is at least 1.1 (assume no TLS if not)
- Change "This application doesn't support HTTPS" errors to "This TCP/IP doesn't support TLS"
- Update application version in user agent header

@Konamiman Konamiman merged commit ef1adda into Konamiman:master Aug 14, 2019

@Konamiman Konamiman deleted the ducasp:ducasp-hget1.3-patch1 branch Aug 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.