Repository: isomorphic-git/isomorphic-git (js)

Issue:
TypeError is thrown during clone when permission is denied to repository on BitBucket
A TypeError is thrown during clone under the following circumstances:

* The repository is hosted on Bitbucket.
* The credentials (user:password) must be valid for an existing account on the server.
* The account must not have access permission to the repository.

The problem seems to be in the capabilities negotiation.

Here's the stack from the error:

```
Caused by: TypeError: Cannot read property 'split' of undefined
    at parseRefsAdResponse (/mnt/c/Users/ecroc/git/external/antora/node_modules/isomorphic-git/dist/for-node/isomorphic-git/index.js:5123:20)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Function.discover (/mnt/c/Users/ecroc/git/external/antora/node_modules/isomorphic-git/dist/for-node/isomorphic-git/index.js:5241:26)
    at async fetchPackfile (/mnt/c/Users/ecroc/git/external/antora/node_modules/isomorphic-git/dist/for-node/isomorphic-git/index.js:6041:22)
    at async fetch (/mnt/c/Users/ecroc/git/external/antora/node_modules/isomorphic-git/dist/for-node/isomorphic-git/index.js:5902:22)
    at async Object.clone (/mnt/c/Users/ecroc/git/external/antora/node_modules/isomorphic-git/dist/for-node/isomorphic-git/index.js:6413:42)
```

Pull request:
resolves #911 handle unexpected response better
This fixes the 3 places in parseRefsAdResponse I see where the result of split(sep) is assumed to have two elements.  I'm throwing a hopefully informative error including the actual problematical response line.

There's one test for missing `\0`, one for missing space, and one for double space.

Currently the error message doesn't display the `\0` separator visibly, is there an easy way to do this?

I'm happy to squash the tests into the code change if desirable.

fixes #911

Patch:
diff --git a/.all-contributorsrc b/.all-contributorsrc
index d8b2f6f3..578372bc 100644
--- a/.all-contributorsrc
+++ b/.all-contributorsrc
@@ -363,6 +363,7 @@
       "avatar_url": "https://avatars2.githubusercontent.com/u/569822?v=4",
       "profile": "https://github.com/djencks",
       "contributions": [
+        "bug",
         "code",
         "test"
       ]
diff --git a/README.md b/README.md
index 414ae112..dc2e2db6 100644
--- a/README.md
+++ b/README.md
@@ -334,7 +334,7 @@ Thanks goes to these wonderful people ([emoji key](https://github.com/kentcdodds
     <td align="center"><a href="https://hen.ne.ke"><img src="https://avatars0.githubusercontent.com/u/4312191?v=4&s=60" width="60px;" alt="Fabian Henneke"/><br /><sub><b>Fabian Henneke</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/issues?q=author%3AFabianHenneke" title="Bug reports">🐛</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=FabianHenneke" title="Code">💻</a></td>
   </tr>
   <tr>
-    <td align="center"><a href="https://github.com/djencks"><img src="https://avatars2.githubusercontent.com/u/569822?v=4" width="60px;" alt="djencks"/><br /><sub><b>djencks</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=djencks" title="Code">💻</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=djencks" title="Tests">⚠️</a></td>
+    <td align="center"><a href="https://github.com/djencks"><img src="https://avatars2.githubusercontent.com/u/569822?v=4" width="60px;" alt="djencks"/><br /><sub><b>djencks</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/issues?q=author%3Adjencks" title="Bug reports">🐛</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=djencks" title="Code">💻</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=djencks" title="Tests">⚠️</a></td>
   </tr>
 </table>
 
diff --git a/src/models/GitTree.js b/src/models/GitTree.js
index d3b01f22..4b194a10 100644
--- a/src/models/GitTree.js
+++ b/src/models/GitTree.js
@@ -84,9 +84,6 @@ export class GitTree {
   constructor (entries) {
     if (Buffer.isBuffer(entries)) {
       this._entries = parseBuffer(entries)
-      // There appears to be an edge case (in this repo no less) where
-      // the tree is NOT sorted as expected if some directories end with ".git"
-      this._entries.sort(comparePath)
     } else if (Array.isArray(entries)) {
       this._entries = entries.map(nudgeIntoShape)
     } else {
@@ -94,6 +91,9 @@ export class GitTree {
         message: 'invalid type passed to GitTree constructor'
       })
     }
+    // There appears to be an edge case (in this repo no less) where
+    // the tree is NOT sorted as expected if some directories end with ".git"
+    this._entries.sort(comparePath)
   }
 
   static from (tree) {
diff --git a/src/models/GitWalkerIndex2.js b/src/models/GitWalkerIndex2.js
index 02316b54..d038088b 100644
--- a/src/models/GitWalkerIndex2.js
+++ b/src/models/GitWalkerIndex2.js
@@ -54,13 +54,9 @@ export class GitWalkerIndex2 {
     if (inode.type !== 'tree') {
       throw new Error(`ENOTDIR: not a directory, scandir '${filepath}'`)
     }
-    return inode.children
-      .map(
-        inode => inode.fullpath
-        // TODO: Figure out why flatFileListToDirectoryStructure is not returning children
-        // sorted correctly for "__tests__/__fixtures__/test-push.git"
-      )
-      .sort((a, b) => compareStrings(a.fullpath, b.fullpath))
+    const names = inode.children.map(inode => inode.fullpath)
+    names.sort(compareStrings)
+    return names
   }
 
   async type (entry) {
diff --git a/src/wire/parseRefsAdResponse.js b/src/wire/parseRefsAdResponse.js
index d94a8e6a..60ba16ee 100644
--- a/src/wire/parseRefsAdResponse.js
+++ b/src/wire/parseRefsAdResponse.js
@@ -26,21 +26,19 @@ export async function parseRefsAdResponse (stream, { service }) {
   // In the edge case of a brand new repo, zero refs (and zero capabilities)
   // are returned.
   if (lineTwo === true) return { capabilities, refs, symrefs }
-  const [firstRef, capabilitiesLine] = lineTwo
-    .toString('utf8')
-    .trim()
-    .split('\x00')
+  const [firstRef, capabilitiesLine] = splitAndAssert(
+    lineTwo.toString('utf8'),
+    '\x00',
+    '\\x00'
+  )
   capabilitiesLine.split(' ').map(x => capabilities.add(x))
-  const [ref, name] = firstRef.split(' ')
+  const [ref, name] = splitAndAssert(firstRef, ' ', ' ')
   refs.set(name, ref)
   while (true) {
     const line = await read()
     if (line === true) break
     if (line !== null) {
-      const [ref, name] = line
-        .toString('utf8')
-        .trim()
-        .split(' ')
+      const [ref, name] = splitAndAssert(line.toString('utf8'), ' ', ' ')
       refs.set(name, ref)
     }
   }
@@ -55,3 +53,14 @@ export async function parseRefsAdResponse (stream, { service }) {
   }
   return { capabilities, refs, symrefs }
 }
+
+function splitAndAssert (line, sep, expected) {
+  const split = line.trim().split(sep)
+  if (split.length !== 2) {
+    throw new GitError(E.AssertServerResponseFail, {
+      expected: `Two strings separated by '${expected}'`,
+      actual: line.toString('utf8')
+    })
+  }
+  return split
+}


Test patch:
diff --git a/__tests__/test-wire.js b/__tests__/test-wire.js
index 89c13475..e97a7ffc 100644
--- a/__tests__/test-wire.js
+++ b/__tests__/test-wire.js
@@ -5,7 +5,8 @@ const {
   parseUploadPackResponse,
   parseUploadPackRequest,
   writeRefsAdResponse,
-  writeUploadPackRequest
+  writeUploadPackRequest,
+  E
 } = require('isomorphic-git/internal-apis')
 // const stream = require('stream')
 
@@ -170,6 +171,110 @@ describe('git wire protocol', () => {
       ['refs/heads/master5', 'e5c144897b64a44bd1164a0db60738452c9eaf87']
     ])
   })
+  it('parseRefsAdResponse bad service', async () => {
+    const res = [
+      Buffer.from(`001e# noservice=git-upload-pack
+000001149ea43b479f5fedc679e3eb37803275d727bf51b7 HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed no-done symref=HEAD:refs/heads/master agent=git/isomorphic-git@0.0.0-development
+003cfb74ea1a9b6a9601df18c38d3de751c51f064bf7 refs/heads/js2
+003c5faa96fe725306e060386975a70e4b6eacb576ed refs/heads/js3
+003f9ea43b479f5fedc679e3eb37803275d727bf51b7 refs/heads/master
+0040c1751a5447a7b025e5bca507af483dde7b0b956f refs/heads/master2
+0040d85135a47c42c9c906e20c08def2fbceac4c2a4f refs/heads/master3
+004018f4b62440abf61285fbfdcbfd990ab8434ff35c refs/heads/master4
+0040e5c144897b64a44bd1164a0db60738452c9eaf87 refs/heads/master5
+0000`)
+    ]
+    try {
+      await parseRefsAdResponse(res, {
+        service: 'git-upload-pack'
+      })
+      fail('expected an error')
+    } catch (error) {
+      expect(error.code).toEqual(E.AssertServerResponseFail)
+      expect(error.data).toEqual({
+        expected: '# service=git-upload-pack\\n',
+        actual: '# noservice=git-upload-pac'
+      })
+    }
+  })
+  it('parseRefsAdResponse bad null separated', async () => {
+    const res = [
+      Buffer.from(`001e# service=git-upload-pack
+0072ERR Repository not found
+The requested repository does not exist, or you do not have permission to
+access it.
+`)
+    ]
+    try {
+      await parseRefsAdResponse(res, {
+        service: 'git-upload-pack'
+      })
+      fail('expected an error')
+    } catch (error) {
+      expect(error.code).toEqual(E.AssertServerResponseFail)
+      expect(error.data).toEqual({
+        expected: `Two strings separated by '\\x00'`,
+        actual:
+          `ERR Repository not found
+The requested repository does not exist, or you do not have permission to
+access it.
+`
+      })
+    }
+  })
+  it('parseRefsAdResponse HEAD bad space separated', async () => {
+    // two spaces instead of one
+    const res = [
+      Buffer.from(`001e# service=git-upload-pack
+000001149ea43b479f5fedc679e3eb37803275d727bf51b7  HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed no-done symref=HEAD:refs/heads/master agent=git/isomorphic-git@0.0.0-development
+003cfb74ea1a9b6a9601df18c38d3de751c51f064bf7 refs/heads/js2
+003c5faa96fe725306e060386975a70e4b6eacb576ed refs/heads/js3
+003f9ea43b479f5fedc679e3eb37803275d727bf51b7 refs/heads/master
+0040c1751a5447a7b025e5bca507af483dde7b0b956f refs/heads/master2
+0040d85135a47c42c9c906e20c08def2fbceac4c2a4f refs/heads/master3
+004018f4b62440abf61285fbfdcbfd990ab8434ff35c refs/heads/master4
+0040e5c144897b64a44bd1164a0db60738452c9eaf87 refs/heads/master5
+0000`)
+    ]
+    try {
+      await parseRefsAdResponse(res, {
+        service: 'git-upload-pack'
+      })
+      fail('expected an error')
+    } catch (error) {
+      expect(error.code).toEqual(E.AssertServerResponseFail)
+      expect(error.data).toEqual({
+        expected: `Two strings separated by ' '`,
+        actual: '9ea43b479f5fedc679e3eb37803275d727bf51b7  HEAD'
+      })
+    }
+  })
+  it('parseRefsAdResponse refs bad space separated', async () => {
+    const res = [
+      Buffer.from(`001e# service=git-upload-pack
+000001149ea43b479f5fedc679e3eb37803275d727bf51b7 HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed no-done symref=HEAD:refs/heads/master agent=git/isomorphic-git@0.0.0-development
+003cfb74ea1a9b6a9601df18c38d3de751c51f064bf7refs/heads/js2
+003c5faa96fe725306e060386975a70e4b6eacb576ed refs/heads/js3
+003f9ea43b479f5fedc679e3eb37803275d727bf51b7 refs/heads/master
+0040c1751a5447a7b025e5bca507af483dde7b0b956f refs/heads/master2
+0040d85135a47c42c9c906e20c08def2fbceac4c2a4f refs/heads/master3
+004018f4b62440abf61285fbfdcbfd990ab8434ff35c refs/heads/master4
+0040e5c144897b64a44bd1164a0db60738452c9eaf87 refs/heads/master5
+0000`)
+    ]
+    try {
+      await parseRefsAdResponse(res, {
+        service: 'git-upload-pack'
+      })
+      fail('expected an error')
+    } catch (error) {
+      expect(error.code).toEqual(E.AssertServerResponseFail)
+      expect(error.data).toEqual({
+        expected: `Two strings separated by ' '`,
+        actual: 'fb74ea1a9b6a9601df18c38d3de751c51f064bf7refs/heads/js2\n0'
+      })
+    }
+  })
   it('writeUploadPackRequest', async () => {
     const req = {
       capabilities: [