asheplyakov/propagator
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Support booting complete ISOs via HTTP #6

Merged
asheplyakov merged 1 commits from http-fulliso-boot into master 2021-11-03 23:00:07 +03:00
Conversation 2 Commits 1 Files Changed 1 +13 -1
asheplyakov commented 2021-09-08 18:39:19 +03:00
Owner
Copy Link

With this patch stage2 can be loaded from any HTTP mirror like this:

automatic=method:http,network:dhcp,server:mirror.yandex.ru,directory=/altlinux/images/p9/simply/aarch64/slinux-live-9.1.1-aarch64.iso stagename=live
With this patch stage2 can be loaded from any HTTP mirror like this: ```bash automatic=method:http,network:dhcp,server:mirror.yandex.ru,directory=/altlinux/images/p9/simply/aarch64/slinux-live-9.1.1-aarch64.iso stagename=live ```
darktemplar reviewed 2021-09-09 16:06:56 +03:00
url.c Outdated
@ -500,0 +512,4 @@
log_message("%s: file is too big (>= %lu), refusing to download",
__func__, ULONG_MAX);
goto out_err;
} else if (*size == 0) {
darktemplar commented 2021-09-09 15:02:59 +03:00
First-time contributor
Copy Link

В случае если реальный размер файла - 0 байт, то *size == 0 и errno == 0, и ошибки нет. Файл, конечно, будет бесполезен, но сообщение об ошибке будет вводящим в заблуждение. Я думаю, если сначала проверить errno, и выдать сообщение что не распознали Content-Length, то потом при *size == 0 можно смело писать в ошибке "файл нулевого размера для загрузки не подойдёт" или что-нибудь другое с подобным смыслом.

В случае если реальный размер файла - 0 байт, то `*size == 0` и `errno == 0`, и ошибки нет. Файл, конечно, будет бесполезен, но сообщение об ошибке будет вводящим в заблуждение. Я думаю, если сначала проверить `errno`, и выдать сообщение что не распознали Content-Length, то потом при `*size == 0` можно смело писать в ошибке "файл нулевого размера для загрузки не подойдёт" или что-нибудь другое с подобным смыслом.
asheplyakov commented 2021-09-09 18:20:41 +03:00
Author
Owner
Copy Link

если сначала проверить errno, и выдать сообщение что не распознали Content-Length, то потом при *size == 0 можно смело писать в ошибке “файл нулевого размера для загрузки не подойдёт”

К сожалению -- нет. Проверка:

/* test_strtoul.c */
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(int argc, char **argv) {
	char *ptr = NULL;
	unsigned long val;
	if (argc != 2) {
		printf("Usage: %s NUMBER-OR-STRING\n", argv[0]);
		return 1;
	}
	errno = 0;
	val = strtoul(argv[1], &ptr, 10);
	if (errno != 0) {
		printf("strtoul '%s' => %lu, error %s (%d)\n",
			argv[1], val, strerror(errno), errno);
	} else {
		printf("strtoul '%s' => %lu, no error\n", argv[1], val);
	}
	return 0;
}

gcc -O2 -Wall -g -o test_strtoul test_strtoul.c
./test_strtoul abra-cadabra
# strtoul 'abra-cadabra' => 0, no error

Сообщение "файл слишком короткий" в таком случае скорее сбивает с толку, чем помогает.

ERRORS
       EINVAL (not in C99) The given base contains an unsupported value.

       ERANGE The resulting value was out of range.

       The  implementation  may also set errno to EINVAL in case
       no conversion was performed (no digits seen, and 0 returned).

glibc выдаёт только ERANGE:

./test_strtoul 100500
# strtoul '100500' => 100500, no error
./test_strtoul `python3 -c "print(2**64)"`
# strtoul '18446744073709551616' => 18446744073709551615, error Numerical result out of range (34)

Поэтому предлагаю так:

  • Если Content-Length не найден - выдать сообщение вида "Couldn't find Content-Length header"
  • Если Content-Length найден, но не является числом: "invalind Content-Length header: %s"
  • Если Content-Length найден, но файл слишком большой: "file %s is too big (>= ULONG_MAX)"
  • Если Content-Length найден, но файл слишком маленький (нулевой длины): "file %s is empty"

Подойдёт?

> если сначала проверить errno, и выдать сообщение что не распознали Content-Length, то потом при `*size == 0` можно смело писать в ошибке “файл нулевого размера для загрузки не подойдёт” К сожалению -- нет. Проверка: ```c /* test_strtoul.c */ #include <errno.h> #include <stdio.h> #include <stdlib.h> #include <string.h> int main(int argc, char **argv) { char *ptr = NULL; unsigned long val; if (argc != 2) { printf("Usage: %s NUMBER-OR-STRING\n", argv[0]); return 1; } errno = 0; val = strtoul(argv[1], &ptr, 10); if (errno != 0) { printf("strtoul '%s' => %lu, error %s (%d)\n", argv[1], val, strerror(errno), errno); } else { printf("strtoul '%s' => %lu, no error\n", argv[1], val); } return 0; } ``` ```bash gcc -O2 -Wall -g -o test_strtoul test_strtoul.c ./test_strtoul abra-cadabra # strtoul 'abra-cadabra' => 0, no error ``` Сообщение "файл слишком короткий" в таком случае скорее сбивает с толку, чем помогает. <pre> ERRORS EINVAL (not in C99) The given base contains an unsupported value. ERANGE The resulting value was out of range. The implementation may also set errno to EINVAL in case no conversion was performed (no digits seen, and 0 returned). </pre> glibc выдаёт только ERANGE: ```bash ./test_strtoul 100500 # strtoul '100500' => 100500, no error ./test_strtoul `python3 -c "print(2**64)"` # strtoul '18446744073709551616' => 18446744073709551615, error Numerical result out of range (34) ``` Поэтому предлагаю так: * Если Content-Length не найден - выдать сообщение вида "Couldn't find Content-Length header" * Если Content-Length найден, но не является числом: "invalind Content-Length header: %s" * Если Content-Length найден, но файл слишком большой: "file %s is too big (>= ULONG_MAX)" * Если Content-Length найден, но файл слишком маленький (нулевой длины): "file %s is empty" Подойдёт?
darktemplar commented 2021-09-09 18:37:18 +03:00
First-time contributor
Copy Link

Подойдёт. Только invalind -> invalid.

По поводу errno Вы правы. А для проверки, что там было не число, можно в коде примера выше заменить if (errno != 0) { на if ((errno != 0) || (ptr == argv[1])) {.

Подойдёт. Только `invalind` -> `invalid`. По поводу `errno` Вы правы. А для проверки, что там было не число, можно в коде примера выше заменить `if (errno != 0) {` на `if ((errno != 0) || (ptr == argv[1])) {`.
darktemplar reviewed 2021-09-14 10:32:49 +03:00
url.c Outdated
@ -500,0 +509,4 @@
* setting errno. On the other hand if the value is too big
* ERANGE is set (and value is truncated to ULONG_MAX)
*/
if (*endptr != '\0') {
darktemplar commented 2021-09-14 10:30:30 +03:00
First-time contributor
Copy Link

А что если будет "Content-Length: 100500\r\n", т.е. будет перевод строки после числа? Тогда *endptr == '\r'. Лучше всё-таки проверять что endptr != buf + strlen(header_content_length), т.е. там не обнаружено числа.

А что если будет "Content-Length: 100500\r\n", т.е. будет перевод строки после числа? Тогда `*endptr == '\r'`. Лучше всё-таки проверять что `endptr != buf + strlen(header_content_length)`, т.е. там не обнаружено числа.
asheplyakov commented 2021-09-15 18:52:43 +03:00
Author
Owner
Copy Link

А что если будет “Content-Length: 100500\r\n”,

Будет плохо.

проверять что endptr != buf + strlen(header_content_length)

это не равносильно "не обнаружено числа".

Content-Length: xyz --> *endptr == 'x'

Также (наверное) следует рассматривать как ошибочные заголовки вида: Content-Length: 1xyz

И тут мне начинает казаться, что слишком много возни ради того, чтобы отличить файл нулевой длины (Content-Length: 0), от неправильного заголовка (Content-Length: , Content-Length: xyz', Content-Length: 12xyz', и т.п.). И хочется вернуть, как было.

> А что если будет “Content-Length: 100500\r\n”, Будет плохо. > проверять что endptr != buf + strlen(header_content_length) это не равносильно "не обнаружено числа". `Content-Length: xyz` --> `*endptr == 'x'` Также (наверное) следует рассматривать как ошибочные заголовки вида: `Content-Length: 1xyz` И тут мне начинает казаться, что слишком много возни ради того, чтобы отличить файл нулевой длины (`Content-Length: 0`), от неправильного заголовка (`Content-Length: `, `Content-Length: xyz', `Content-Length: 12xyz', и т.п.). И хочется вернуть, как было.
asheplyakov commented 2021-09-15 18:55:48 +03:00
Author
Owner
Copy Link

Грр. gitea съедает пробелы там, где они существенны.

Я хотел сказать, что при заголовке Content-Length: \ xyz endptr будет указывать на символ после второго пробела, и условие endptr != buf + strlen(header_content_length) выполнится, а числа в заголовке нет.

Грр. gitea съедает пробелы там, где они существенны. Я хотел сказать, что при заголовке `Content-Length: \ xyz` endptr будет указывать на символ после второго пробела, и условие `endptr != buf + strlen(header_content_length)` выполнится, а числа в заголовке нет.
darktemplar commented 2021-09-16 11:41:29 +03:00
First-time contributor
Copy Link

Как насчёт такого кода?

int ok = 1;
char *checkptr = endptr;

for ( ; *checkptr && ok ; ++checkptr)
{
    if (!isspace(*checkptr))
    {
        ok = 0;
    }
}

if ((endptr == buf + strlen(header_content_length)) || !ok) {
    // invalid content-length: contains something except one number and space characters
}
else
...

Входные данные и результат:
10 -> good
0 -> good
-> endptr == buf + strlen(header_content_length)
a10 -> endptr == buf + strlen(header_content_length)
10a -> !ok
1 2 3 -> !ok

Как насчёт такого кода? ``` int ok = 1; char *checkptr = endptr; for ( ; *checkptr && ok ; ++checkptr) { if (!isspace(*checkptr)) { ok = 0; } } if ((endptr == buf + strlen(header_content_length)) || !ok) { // invalid content-length: contains something except one number and space characters } else ... ``` Входные данные и результат: ` 10 ` -> good ` 0 ` -> good ` ` -> endptr == buf + strlen(header_content_length) ` a10 ` -> endptr == buf + strlen(header_content_length) ` 10a ` -> !ok ` 1 2 3` -> !ok
darktemplar commented 2021-09-21 14:32:31 +03:00
First-time contributor
Copy Link

Тест - это хорошо. Прошу добавить в спеке секцию %check и запуск тестов в этой секции.

Тест - это хорошо. Прошу добавить в спеке секцию %check и запуск тестов в этой секции.
darktemplar commented 2021-09-23 10:33:40 +03:00
First-time contributor
Copy Link

LGTM

LGTM
asheplyakov merged commit 43a9d78fdd into master 2021-09-24 14:53:08 +03:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: asheplyakov/propagator#6
No description provided.
Delete Branch "http-fulliso-boot"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?