Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

client: ssh communication broken while fetching #80

Closed
mcuadros opened this issue Sep 27, 2016 · 3 comments
Closed

client: ssh communication broken while fetching #80

mcuadros opened this issue Sep 27, 2016 · 3 comments
Assignees
Labels
Milestone

Comments

@mcuadros
Copy link
Contributor

----------------------------------------------------------------------
FAIL: git_upload_pack_test.go:114: RemoteSuite.TestFetch
git_upload_pack_test.go:129:
    //this is failling randomly, should be fixed ASAP
    c.Check(len(b), Equals, 85585)
... obtained int = 73727
... expected int = 85585
OOPS: 20 passed, 1 FAILED
--- FAIL: Test (0.72s)
@mcuadros mcuadros added the bug label Sep 27, 2016
@mcuadros mcuadros added this to the v4 milestone Sep 27, 2016
@ferhatelmas
Copy link
Contributor

@mcuadros I'm taking it, will investigate at weekend.

@mcuadros
Copy link
Contributor Author

mcuadros commented Oct 7, 2016

Sorry but this is being handle by @alcortesm, sorry

@ferhatelmas
Copy link
Contributor

No worries.

If you could point me another getting used to ticket, it will be appreciated. Thanks!

mcuadros pushed a commit that referenced this issue Oct 27, 2016
* Fix #80

We were sending an additional `\n` to the server when sending the
upload request, see clients/ssh/git_upload_pack.go:174:

    fmt.Fprintln(si, r.String())

The reason for this was to flush the input stream, otherwise, the
message was not send to the server.

Also, we were (and still are) not checking remote execution errors, so
we were unaware of this error, reading the first portion of the packfile
as if nothing were wrong.

On the few ocasions when the server was quick enough to fail before
sending the full packfile, one of our tests (the one that checks the
received packfile size) failed.

We were also escaping the repository name in the remote command
execution string incorrectly.

Now we are:

- using ssh.Run to run the remote command, instead of start and wait,
  which is the same but simpler.

- using io.Copy instead of fmt.Fprintln, so we avoid adding and extra EOL and also we don't use a line buffered stream.
  and we no longer have to flush it.

- we are closing the input stream as soon as possible, so the remote
  command can exit also as soon as possible.

- the remote command escape character (') is used correctly

* WIP

* ssh: return remote command exit value when closing the packfile stream
mcuadros pushed a commit that referenced this issue Jan 31, 2017
* Fix #80

We were sending an additional `\n` to the server when sending the
upload request, see clients/ssh/git_upload_pack.go:174:

    fmt.Fprintln(si, r.String())

The reason for this was to flush the input stream, otherwise, the
message was not send to the server.

Also, we were (and still are) not checking remote execution errors, so
we were unaware of this error, reading the first portion of the packfile
as if nothing were wrong.

On the few ocasions when the server was quick enough to fail before
sending the full packfile, one of our tests (the one that checks the
received packfile size) failed.

We were also escaping the repository name in the remote command
execution string incorrectly.

Now we are:

- using ssh.Run to run the remote command, instead of start and wait,
  which is the same but simpler.

- using io.Copy instead of fmt.Fprintln, so we avoid adding and extra EOL and also we don't use a line buffered stream.
  and we no longer have to flush it.

- we are closing the input stream as soon as possible, so the remote
  command can exit also as soon as possible.

- the remote command escape character (') is used correctly

* WIP

* ssh: return remote command exit value when closing the packfile stream
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants