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

Explicitly specify dice server port #3449

Merged

Conversation

ssoloff
Copy link
Member

@ssoloff ssoloff commented Jun 5, 2018

As discussed on the forum, there is a compatibility issue between the latest and current stable clients. #3277 set the scheme property value to https for all MARTI servers. This property is persisted in a save game created by the latest pre-release, and thus read when the save game is opened by any other client.

When the save game is opened in a stable client, the client attempts to open an HTTPS connection to the dice server using its default port of 80. As this port is bound to the dice server's HTTP endpoint, the TLS connection fails.

Alternatively, when the save game is opened in a pre-release client, the client attempts to open an HTTPS connection to the dice server using its default port of -1, which instructs the HTTP stack to use the standard default port for the specified scheme (443 in this case). Thus, the TLS connection succeeds.

The fix is to explicitly specify the port as 443 in the dice server properties files. This ensures that save games created by a pre-release client will have the correct dice server port instead of relying on the default port configured in code. Thus, when such a save game is opened by a stable client, it will behave just as if it were opened by a pre-release client.

Testing

I created a PBF save game using master and verified I could reproduce the TLS error reported above when opening the save game in 9678. I then created a PBF save game using this branch and verified testing the dice server worked when opening the save game in 9678.

Notes

This change can be reverted upon the next incompatible release.

As discussed in [1], there is a compatibility issue between the latest
and current stable clients.  triplea-game#3277 set the "scheme" property value to
"https" for all MARTI servers.  This property is persisted in a save
game created by the latest pre-release, and thus read when the save game
is opened by any other client.

When the save game is opened in a stable client, the client attempts to
open an HTTPS connection to the dice server using its default port of
"80".  As this port is bound to the dice server's HTTP endpoint, the TLS
connection fails.

Alternatively, when the save game is opened in a pre-release client, the
client attempts to open an HTTPS connection to the dice server using its
default port of "-1", which instructs the HTTP stack to use the standard
default port for the specified scheme (443 in this case).  Thus, the TLS
connection succeeds.

The fix is to explicitly specify the port as 443 in the dice server
properties files.  This ensures that save games created by a pre-release
client will have the correct dice server port instead of relying on the
default port configured in code.  Thus, when such a save game is opened
by a stable client, it will behave just as if it were opened by a
pre-release client.

[1] https://forums.triplea-game.org/topic/879/civil-war-3-2-4-redrum-confederate-vs-wirkey-union-2
@codecov-io
Copy link

Codecov Report

Merging #3449 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             master   #3449      +/-   ##
===========================================
+ Coverage     21.98%     22%   +0.01%     
- Complexity     5913    5916       +3     
===========================================
  Files           832     832              
  Lines         71907   71907              
  Branches      11584   11584              
===========================================
+ Hits          15812   15820       +8     
+ Misses        54008   54002       -6     
+ Partials       2087    2085       -2
Impacted Files Coverage Δ Complexity Δ
.../src/main/java/games/strategy/net/nio/Decoder.java 72.82% <0%> (-1.09%) 24% <0%> (-1%)
.../main/java/games/strategy/net/ClientMessenger.java 35.25% <0%> (+0.71%) 14% <0%> (+1%) ⬆️
.../strategy/triplea/odds/calculator/DummyPlayer.java 36.36% <0%> (+1.13%) 10% <0%> (+1%) ⬆️
...rc/main/java/games/strategy/net/nio/NioWriter.java 71.92% <0%> (+1.75%) 20% <0%> (ø) ⬇️
...tegy/engine/message/unifiedmessenger/EndPoint.java 77.46% <0%> (+4.22%) 15% <0%> (+1%) ⬆️
...rategy/triplea/attachments/UnitTypeComparator.java 53.84% <0%> (+7.69%) 13% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0c2cec...9be9bdb. Read the comment docs.

@RoiEXLab RoiEXLab merged commit feee3c4 into triplea-game:master Jun 5, 2018
@ssoloff ssoloff deleted the explicitly-specify-dice-server-port branch June 5, 2018 18:27
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.

None yet

3 participants