Skip to content

Commit e15c418

Browse files
committed
Fix off by 1 problem.
The problem was manifestated only with BIT columns and only when more than one row was fetched. The problem was coming from the fact that in pre-7.0 times mysqlnd was using a no-copy optimization. This optimization kept the strings (and also the BIT mask equivalents as strings) in the packet and the zval referred to them. 7.0+ zvals cannot use no-copy and always copy. Because of this the allocated memory for the packet was reduced by 1 by the person who ported the driver, but the starting address of the bit area wasn't reduced. Because of this the bit_area started at wrong address and the length decoded wrong.
1 parent 7a8774a commit e15c418

File tree

3 files changed

+33
-3
lines changed

3 files changed

+33
-3
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ PHP NEWS
1212
. Fixed bug #73679 (DOTNET read access violation using invalid codepage).
1313
(Anatol)
1414

15+
- Mysqlnd:
16+
. Fixed issue with decoding BIT columns when having more than one rows in the
17+
result set. 7.0+ problem. (Andrey)
18+
1519
- PCRE:
1620
. Fixed bug #73612 (preg_*() may leak memory). (cmb)
1721

ext/mysqlnd/mysqlnd_ps_codec.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ ps_fetch_from_1_to_8_bytes(zval * zv, const MYSQLND_FIELD * const field, unsigne
8888
} else {
8989
DBG_INF("stringify");
9090
tmp_len = sprintf((char *)&tmp, MYSQLND_LLU_SPEC, uval);
91+
DBG_INF_FMT("value=%s", tmp);
9192
}
9293
}
9394
} else {

ext/mysqlnd/mysqlnd_wireprotocol.c

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,7 +1607,8 @@ php_mysqlnd_rowp_read_text_protocol_aux(MYSQLND_MEMORY_POOL_CHUNK * row_buffer,
16071607
zval *current_field, *end_field, *start_field;
16081608
zend_uchar * p = row_buffer->ptr;
16091609
size_t data_size = row_buffer->app;
1610-
zend_uchar * bit_area = (zend_uchar*) row_buffer->ptr + data_size + 1; /* we allocate from here */
1610+
/* we allocate from here. In pre-7.0 it was +1, as there was an additional \0 for the last string in the packet - because of the zval optimizations - using no-copy */
1611+
zend_uchar * bit_area = (zend_uchar*) row_buffer->ptr + data_size;
16111612
const zend_uchar * const packet_end = (zend_uchar*) row_buffer->ptr + data_size;
16121613

16131614
DBG_ENTER("php_mysqlnd_rowp_read_text_protocol_aux");
@@ -1734,9 +1735,25 @@ php_mysqlnd_rowp_read_text_protocol_aux(MYSQLND_MEMORY_POOL_CHUNK * row_buffer,
17341735
*/
17351736
p -= len;
17361737
if (Z_TYPE_P(current_field) == IS_LONG) {
1738+
/*
1739+
Andrey : See below. No need of bit_area, as we can use on stack for this.
1740+
The bit area should be removed - the `prealloc_more_bytes` in php_mysqlnd_read_row_ex()
1741+
1742+
char tmp[22];
1743+
const size_t tmp_len = sprintf((char *)&tmp, MYSQLND_LLU_SPEC, Z_LVAL_P(current_field));
1744+
ZVAL_STRINGL(current_field, tmp, tmp_len);
1745+
*/
17371746
bit_area += 1 + sprintf((char *)start, ZEND_LONG_FMT, Z_LVAL_P(current_field));
17381747
ZVAL_STRINGL(current_field, (char *) start, bit_area - start - 1);
1739-
} else if (Z_TYPE_P(current_field) == IS_STRING){
1748+
} else if (Z_TYPE_P(current_field) == IS_STRING) {
1749+
/*
1750+
Andrey : This is totally sensless, but I am not gonna remove it in a production version.
1751+
This copies the data from the zval to the bit area. The destroys the original value
1752+
and creates the same one from the bit area. No need. It was making sense in pre-7.0
1753+
when we used zval IS_STRING with no-copy that referred to the bit area.
1754+
The bit area has no sense in both the case of IS_LONG and IS_STRING as 7.0 zval
1755+
IS_STRING always copies.
1756+
*/
17401757
memcpy(bit_area, Z_STRVAL_P(current_field), Z_STRLEN_P(current_field));
17411758
bit_area += Z_STRLEN_P(current_field);
17421759
*bit_area++ = '\0';
@@ -1815,7 +1832,15 @@ php_mysqlnd_rowp_read(void * _packet, MYSQLND_CONN_DATA * conn)
18151832
packet_type_to_statistic_packet_count[PROT_ROW_PACKET],
18161833
1);
18171834

1818-
/* packet->row_buffer->ptr is of size 'data_size + 1' */
1835+
/*
1836+
packet->row_buffer->ptr is of size 'data_size'
1837+
in pre-7.0 it was really 'data_size + 1' although it was counted as 'data_size'
1838+
The +1 was for the additional byte needed to \0 terminate the last string in the row.
1839+
This was needed as the zvals of pre-7.0 could use external memory (no copy param to ZVAL_STRINGL).
1840+
However, in 7.0+ the strings always copy. Thus this +1 byte was removed. Also the optimization or \0
1841+
terminating every string, which did overwrite the lengths from the packet. For this reason we needed
1842+
to keep (and copy) the lengths externally.
1843+
*/
18191844
packet->header.size = data_size;
18201845
packet->row_buffer->app = data_size;
18211846

0 commit comments

Comments
 (0)