Fix really bad error handling in git_smart__negotiate_fetch
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index af26ee5..5896215 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -231,10 +231,10 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
git_oid oid;
/* No own logic, do our thing */
- if (git_pkt_buffer_wants(refs, count, &t->caps, &data) < 0)
- return -1;
+ if ((error = git_pkt_buffer_wants(refs, count, &t->caps, &data)) < 0)
+ return error;
- if (fetch_setup_walk(&walk, repo) < 0)
+ if ((error = fetch_setup_walk(&walk, repo)) < 0)
goto on_error;
/*
* We don't support any kind of ACK extensions, so the negotiation
@@ -242,7 +242,16 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
* every once in a while.
*/
i = 0;
- while ((error = git_revwalk_next(&oid, walk)) == 0) {
+ while (true) {
+ error = git_revwalk_next(&oid, walk);
+
+ if (error < 0) {
+ if (GIT_ITEROVER == error)
+ break;
+
+ goto on_error;
+ }
+
git_pkt_buffer_have(&oid, &data);
i++;
if (i % 20 == 0) {
@@ -253,15 +262,17 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
}
git_pkt_buffer_flush(&data);
- if (git_buf_oom(&data))
+ if (git_buf_oom(&data)) {
+ error = -1;
goto on_error;
+ }
- if (git_smart__negotiation_step(&t->parent, data.ptr, data.size) < 0)
+ if ((error = git_smart__negotiation_step(&t->parent, data.ptr, data.size)) < 0)
goto on_error;
git_buf_clear(&data);
if (t->caps.multi_ack) {
- if (store_common(t) < 0)
+ if ((error = store_common(t)) < 0)
goto on_error;
} else {
pkt_type = recv_pkt(NULL, buf);
@@ -270,8 +281,13 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
break;
} else if (pkt_type == GIT_PKT_NAK) {
continue;
+ } else if (pkt_type < 0) {
+ /* recv_pkt returned an error */
+ error = pkt_type;
+ goto on_error;
} else {
giterr_set(GITERR_NET, "Unexpected pkt type");
+ error = -1;
goto on_error;
}
}
@@ -284,44 +300,49 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
git_pkt_ack *pkt;
unsigned int i;
- if (git_pkt_buffer_wants(refs, count, &t->caps, &data) < 0)
+ if ((error = git_pkt_buffer_wants(refs, count, &t->caps, &data)) < 0)
goto on_error;
git_vector_foreach(&t->common, i, pkt) {
- git_pkt_buffer_have(&pkt->oid, &data);
+ if ((error = git_pkt_buffer_have(&pkt->oid, &data)) < 0)
+ goto on_error;
}
- if (git_buf_oom(&data))
+ if (git_buf_oom(&data)) {
+ error = -1;
goto on_error;
+ }
}
}
- if (error < 0 && error != GIT_ITEROVER)
- goto on_error;
-
/* Tell the other end that we're done negotiating */
if (t->rpc && t->common.length > 0) {
git_pkt_ack *pkt;
unsigned int i;
- if (git_pkt_buffer_wants(refs, count, &t->caps, &data) < 0)
+ if ((error = git_pkt_buffer_wants(refs, count, &t->caps, &data)) < 0)
goto on_error;
git_vector_foreach(&t->common, i, pkt) {
- git_pkt_buffer_have(&pkt->oid, &data);
+ if ((error = git_pkt_buffer_have(&pkt->oid, &data)) < 0)
+ goto on_error;
}
- if (git_buf_oom(&data))
+ if (git_buf_oom(&data)) {
+ error = -1;
goto on_error;
+ }
}
- git_pkt_buffer_done(&data);
+ if ((error = git_pkt_buffer_done(&data)) < 0)
+ goto on_error;
+
if (t->cancelled.val) {
giterr_set(GITERR_NET, "The fetch was cancelled by the user");
error = GIT_EUSER;
goto on_error;
}
- if (git_smart__negotiation_step(&t->parent, data.ptr, data.size) < 0)
+ if ((error = git_smart__negotiation_step(&t->parent, data.ptr, data.size)) < 0)
goto on_error;
git_buf_free(&data);
@@ -330,15 +351,18 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
/* Now let's eat up whatever the server gives us */
if (!t->caps.multi_ack) {
pkt_type = recv_pkt(NULL, buf);
- if (pkt_type != GIT_PKT_ACK && pkt_type != GIT_PKT_NAK) {
+
+ if (pkt_type < 0) {
+ return pkt_type;
+ } else if (pkt_type != GIT_PKT_ACK && pkt_type != GIT_PKT_NAK) {
giterr_set(GITERR_NET, "Unexpected pkt type");
return -1;
}
} else {
git_pkt_ack *pkt;
do {
- if (recv_pkt((git_pkt **)&pkt, buf) < 0)
- return -1;
+ if ((error = recv_pkt((git_pkt **)&pkt, buf)) < 0)
+ return error;
if (pkt->type == GIT_PKT_NAK ||
(pkt->type == GIT_PKT_ACK && pkt->status != GIT_ACK_CONTINUE)) {