From b65a1d7edc5c986b6ad5cbd586b92f6bedc28619 Mon Sep 17 00:00:00 2001 From: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Mon, 20 Feb 2023 13:16:24 -0800 Subject: [PATCH 1/8] Create PR template (#154) --- .github/pull_request_template.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..946c1f63 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,21 @@ + + +Description +----------- + + +Test Steps +----------- + + +Checklist: +---------- + + +- [ ] I have tested my changes. No regression in existing tests. +- [ ] I have modified and/or added unit-tests to cover the code changes in this Pull Request. + +Related Issue +----------- + +By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. From e72c60e75b5909f0ec7eb4725a2092d5cd298640 Mon Sep 17 00:00:00 2001 From: Kareem Khazem Date: Thu, 9 Mar 2023 18:30:50 +0000 Subject: [PATCH 2/8] Do not run InitializeRequestHeaders and ParserOnBodyCallback This commit stops two proofs from running in CI. --- .../proofs/HTTPClient_InitializeRequestHeaders/cbmc-proof.txt | 1 - test/cbmc/proofs/httpParserOnBodyCallback/cbmc-proof.txt | 1 - 2 files changed, 2 deletions(-) delete mode 100644 test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/cbmc-proof.txt delete mode 100644 test/cbmc/proofs/httpParserOnBodyCallback/cbmc-proof.txt diff --git a/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/cbmc-proof.txt b/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/cbmc-proof.txt deleted file mode 100644 index 6ed46f12..00000000 --- a/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/cbmc-proof.txt +++ /dev/null @@ -1 +0,0 @@ -# This file marks this directory as containing a CBMC proof. diff --git a/test/cbmc/proofs/httpParserOnBodyCallback/cbmc-proof.txt b/test/cbmc/proofs/httpParserOnBodyCallback/cbmc-proof.txt deleted file mode 100644 index 6ed46f12..00000000 --- a/test/cbmc/proofs/httpParserOnBodyCallback/cbmc-proof.txt +++ /dev/null @@ -1 +0,0 @@ -# This file marks this directory as containing a CBMC proof. From 48ccceb4bc048a36012ff33a0adbb49bc891045f Mon Sep 17 00:00:00 2001 From: Kareem Khazem Date: Tue, 28 Feb 2023 22:19:31 +0000 Subject: [PATCH 3/8] Add CBMC-running GitHub Action; This commit adds a GitHub Action that runs the CBMC proofs in this repository upon pushes and pull requests --- .github/workflows/ci.yml | 16 ++++ .../cbmc/proofs/HTTPClient_AddHeader/Makefile | 2 + .../proofs/HTTPClient_AddRangeHeader/Makefile | 2 + .../Makefile | 2 + .../proofs/HTTPClient_ReadHeader/Makefile | 2 + test/cbmc/proofs/HTTPClient_Send/Makefile | 2 + test/cbmc/proofs/HTTPClient_strerror/Makefile | 2 + .../findHeaderFieldParserCallback/Makefile | 2 + .../Makefile | 2 + .../findHeaderValueParserCallback/Makefile | 2 + .../proofs/httpParserOnBodyCallback/Makefile | 2 + .../httpParserOnHeaderFieldCallback/Makefile | 2 + .../httpParserOnHeaderValueCallback/Makefile | 2 + .../Makefile | 2 + .../httpParserOnMessageBeginCallback/Makefile | 2 + .../Makefile | 2 + .../httpParserOnStatusCallback/Makefile | 2 + test/cbmc/proofs/lib/print_tool_versions.py | 74 +++++++++++++++++++ test/cbmc/proofs/lib/summarize.py | 50 +++++++++---- test/cbmc/proofs/run-cbmc-proofs.py | 28 ++----- 20 files changed, 166 insertions(+), 34 deletions(-) create mode 100755 test/cbmc/proofs/lib/print_tool_versions.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3e58fe14..e2d51615 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,3 +124,19 @@ jobs: with: config: .github/memory_statistics_config.json check_against: docs/doxygen/include/size_table.md + proof_ci: + runs-on: cbmc_ubuntu-latest_64-core + steps: + - name: Set up CBMC runner + uses: FreeRTOS/CI-CD-Github-Actions/set_up_cbmc_runner@main + with: + kissat_tag: latest + cbmc_version: "5.73.0" + - run: | + git submodule update --init --recursive --checkout + sudo apt-get update + sudo apt-get install --yes --no-install-recommends gcc-multilib build-essential + - name: Run CBMC + uses: FreeRTOS/CI-CD-Github-Actions/run_cbmc@main + with: + proofs_dir: test/cbmc/proofs diff --git a/test/cbmc/proofs/HTTPClient_AddHeader/Makefile b/test/cbmc/proofs/HTTPClient_AddHeader/Makefile index 7394ec1f..a9bd0f8b 100644 --- a/test/cbmc/proofs/HTTPClient_AddHeader/Makefile +++ b/test/cbmc/proofs/HTTPClient_AddHeader/Makefile @@ -58,4 +58,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/httpHeaderStrncpy.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/HTTPClient_AddRangeHeader/Makefile b/test/cbmc/proofs/HTTPClient_AddRangeHeader/Makefile index 71fcf7c5..20bfa6a9 100644 --- a/test/cbmc/proofs/HTTPClient_AddRangeHeader/Makefile +++ b/test/cbmc/proofs/HTTPClient_AddRangeHeader/Makefile @@ -48,4 +48,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/httpHeaderStrncpy.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/Makefile b/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/Makefile index e8c8208d..509eeb59 100644 --- a/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/Makefile +++ b/test/cbmc/proofs/HTTPClient_InitializeRequestHeaders/Makefile @@ -57,4 +57,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/httpHeaderStrncpy.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/HTTPClient_ReadHeader/Makefile b/test/cbmc/proofs/HTTPClient_ReadHeader/Makefile index f96dbc4e..f5155dc7 100644 --- a/test/cbmc/proofs/HTTPClient_ReadHeader/Makefile +++ b/test/cbmc/proofs/HTTPClient_ReadHeader/Makefile @@ -44,4 +44,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/HTTPClient_ReadHeader_llhttp_execute. PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/HTTPClient_Send/Makefile b/test/cbmc/proofs/HTTPClient_Send/Makefile index 49273bde..931729ad 100644 --- a/test/cbmc/proofs/HTTPClient_Send/Makefile +++ b/test/cbmc/proofs/HTTPClient_Send/Makefile @@ -74,4 +74,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/httpHeaderStrncpy.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/HTTPClient_strerror/Makefile b/test/cbmc/proofs/HTTPClient_strerror/Makefile index 05dfe5b5..ea3c41e3 100644 --- a/test/cbmc/proofs/HTTPClient_strerror/Makefile +++ b/test/cbmc/proofs/HTTPClient_strerror/Makefile @@ -33,4 +33,6 @@ PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE).c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/findHeaderFieldParserCallback/Makefile b/test/cbmc/proofs/findHeaderFieldParserCallback/Makefile index d6a98ee0..57d26c84 100644 --- a/test/cbmc/proofs/findHeaderFieldParserCallback/Makefile +++ b/test/cbmc/proofs/findHeaderFieldParserCallback/Makefile @@ -20,4 +20,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/findHeaderOnHeaderCompleteCallback/Makefile b/test/cbmc/proofs/findHeaderOnHeaderCompleteCallback/Makefile index 8df45dad..f39923a9 100644 --- a/test/cbmc/proofs/findHeaderOnHeaderCompleteCallback/Makefile +++ b/test/cbmc/proofs/findHeaderOnHeaderCompleteCallback/Makefile @@ -16,4 +16,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/findHeaderValueParserCallback/Makefile b/test/cbmc/proofs/findHeaderValueParserCallback/Makefile index 53ca9f71..226b1f8e 100644 --- a/test/cbmc/proofs/findHeaderValueParserCallback/Makefile +++ b/test/cbmc/proofs/findHeaderValueParserCallback/Makefile @@ -16,4 +16,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnBodyCallback/Makefile b/test/cbmc/proofs/httpParserOnBodyCallback/Makefile index 77d96812..9e3f8697 100644 --- a/test/cbmc/proofs/httpParserOnBodyCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnBodyCallback/Makefile @@ -18,4 +18,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/memmove.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnHeaderFieldCallback/Makefile b/test/cbmc/proofs/httpParserOnHeaderFieldCallback/Makefile index e7c3635b..9ea2d735 100644 --- a/test/cbmc/proofs/httpParserOnHeaderFieldCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnHeaderFieldCallback/Makefile @@ -17,4 +17,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/callback_stubs.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnHeaderValueCallback/Makefile b/test/cbmc/proofs/httpParserOnHeaderValueCallback/Makefile index 8cae4cf9..5af9f46c 100644 --- a/test/cbmc/proofs/httpParserOnHeaderValueCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnHeaderValueCallback/Makefile @@ -16,4 +16,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnHeadersCompleteCallback/Makefile b/test/cbmc/proofs/httpParserOnHeadersCompleteCallback/Makefile index 8c42da25..cc5f3836 100644 --- a/test/cbmc/proofs/httpParserOnHeadersCompleteCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnHeadersCompleteCallback/Makefile @@ -17,4 +17,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/stubs/callback_stubs.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnMessageBeginCallback/Makefile b/test/cbmc/proofs/httpParserOnMessageBeginCallback/Makefile index 4b9e17dc..4441f791 100644 --- a/test/cbmc/proofs/httpParserOnMessageBeginCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnMessageBeginCallback/Makefile @@ -16,4 +16,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnMessageCompleteCallback/Makefile b/test/cbmc/proofs/httpParserOnMessageCompleteCallback/Makefile index cd3074e3..dc76ba48 100644 --- a/test/cbmc/proofs/httpParserOnMessageCompleteCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnMessageCompleteCallback/Makefile @@ -16,4 +16,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/httpParserOnStatusCallback/Makefile b/test/cbmc/proofs/httpParserOnStatusCallback/Makefile index da847919..f0454bf6 100644 --- a/test/cbmc/proofs/httpParserOnStatusCallback/Makefile +++ b/test/cbmc/proofs/httpParserOnStatusCallback/Makefile @@ -16,4 +16,6 @@ PROOF_SOURCES += $(SRCDIR)/test/cbmc/sources/http_cbmc_state.c PROJECT_SOURCES += $(SRCDIR)/source/core_http_client.c +EXTERNAL_SAT_SOLVER := kissat + include ../Makefile.common diff --git a/test/cbmc/proofs/lib/print_tool_versions.py b/test/cbmc/proofs/lib/print_tool_versions.py new file mode 100755 index 00000000..bdeb429e --- /dev/null +++ b/test/cbmc/proofs/lib/print_tool_versions.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +# +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 + + +import logging +import pathlib +import shutil +import subprocess + + +_TOOLS = [ + "cadical", + "cbmc", + "cbmc-viewer", + "cbmc-starter-kit-update", + "kissat", + "litani", +] + + +def _format_versions(table): + lines = [ + "", + '', + ] + for tool, version in table.items(): + if version: + v_str = f'
{version}
' + else: + v_str = 'not found' + lines.append( + f'' + f'') + lines.append("
Tool Versions
{tool}:{v_str}
") + return "\n".join(lines) + + +def _get_tool_versions(): + ret = {} + for tool in _TOOLS: + err = f"Could not determine version of {tool}: " + ret[tool] = None + if not shutil.which(tool): + logging.error("%s'%s' not found on $PATH", err, tool) + continue + cmd = [tool, "--version"] + proc = subprocess.Popen(cmd, text=True, stdout=subprocess.PIPE) + try: + out, _ = proc.communicate(timeout=10) + except subprocess.TimeoutExpired: + logging.error("%s'%s --version' timed out", err, tool) + continue + if proc.returncode: + logging.error( + "%s'%s --version' returned %s", err, tool, str(proc.returncode)) + continue + ret[tool] = out.strip() + return ret + + +def main(): + exe_name = pathlib.Path(__file__).name + logging.basicConfig(format=f"{exe_name}: %(message)s") + + table = _get_tool_versions() + out = _format_versions(table) + print(out) + + +if __name__ == "__main__": + main() diff --git a/test/cbmc/proofs/lib/summarize.py b/test/cbmc/proofs/lib/summarize.py index a5fec96f..50dbcc33 100644 --- a/test/cbmc/proofs/lib/summarize.py +++ b/test/cbmc/proofs/lib/summarize.py @@ -4,6 +4,8 @@ import argparse import json import logging +import os +import sys DESCRIPTION = """Print 2 tables in GitHub-flavored Markdown that summarize @@ -89,8 +91,9 @@ def _get_status_and_proof_summaries(run_dict): count_statuses[status_pretty_name] += 1 except KeyError: count_statuses[status_pretty_name] = 1 - proof = proof_pipeline["name"] - proofs.append([proof, status_pretty_name]) + if proof_pipeline["name"] == "print_tool_versions": + continue + proofs.append([proof_pipeline["name"], status_pretty_name]) statuses = [["Status", "Count"]] for status, count in count_statuses.items(): statuses.append([status, str(count)]) @@ -102,18 +105,39 @@ def print_proof_results(out_file): Print 2 strings that summarize the proof results. When printing, each string will render as a GitHub flavored Markdown table. """ - print("## Summary of CBMC proof results") - try: - with open(out_file, encoding='utf-8') as run_json: - run_dict = json.load(run_json) - summaries = _get_status_and_proof_summaries( - run_dict) - for summary in summaries: - print(_get_rendered_table(summary)) - except Exception as ex: # pylint: disable=broad-except - logging.critical("Could not print results. Exception: %s", str(ex)) + output = "## Summary of CBMC proof results\n\n" + with open(out_file, encoding='utf-8') as run_json: + run_dict = json.load(run_json) + status_table, proof_table = _get_status_and_proof_summaries(run_dict) + for summary in (status_table, proof_table): + output += _get_rendered_table(summary) + + print(output) + sys.stdout.flush() + + github_summary_file = os.getenv("GITHUB_STEP_SUMMARY") + if github_summary_file: + with open(github_summary_file, "a") as handle: + print(output, file=handle) + handle.flush() + else: + logging.warning( + "$GITHUB_STEP_SUMMARY not set, not writing summary file") + + msg = ( + "Click the 'Summary' button to view a Markdown table " + "summarizing all proof results") + if run_dict["status"] != "success": + logging.error("Not all proofs passed.") + logging.error(msg) + sys.exit(1) + logging.info(msg) if __name__ == '__main__': args = get_args() - print_proof_results(args.run_file) + logging.basicConfig(format="%(levelname)s: %(message)s") + try: + print_proof_results(args.run_file) + except Exception as ex: # pylint: disable=broad-except + logging.critical("Could not print results. Exception: %s", str(ex)) diff --git a/test/cbmc/proofs/run-cbmc-proofs.py b/test/cbmc/proofs/run-cbmc-proofs.py index c172de31..3882b67b 100755 --- a/test/cbmc/proofs/run-cbmc-proofs.py +++ b/test/cbmc/proofs/run-cbmc-proofs.py @@ -223,7 +223,7 @@ def run_build(litani, jobs, fail_on_proof_failure, summarize): cmd.extend(["--out-file", str(out_file)]) logging.debug(" ".join(cmd)) - proc = subprocess.run(cmd, check=False) + proc = subprocess.run(cmd, check=False, timeout=360) if proc.returncode and not fail_on_proof_failure: logging.critical("Failed to run litani run-build") @@ -313,11 +313,15 @@ async def configure_proof_dirs( # pylint: disable=too-many-arguments profiling = [ "ENABLE_MEMORY_PROFILING=true"] if enable_memory_profiling else [] + env = dict(os.environ) + env["EXTERNAL_SAT_SOLVER"] = "kissat" + # Allow interactive tasks to preempt proof configuration proc = await asyncio.create_subprocess_exec( - "nice", "-n", "15", "make", *pools, + "nice", "-n", "15", "make", "EXTERNAL_SAT_SOLVER=kissat", *pools, *profiling, "-B", report_target, "" if debug else "--quiet", cwd=path, - stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE) + stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + env=env) stdout, stderr = await proc.communicate() logging.debug("returncode: %s", str(proc.returncode)) logging.debug("stdout:") @@ -334,22 +338,6 @@ async def configure_proof_dirs( # pylint: disable=too-many-arguments queue.task_done() -def add_tool_version_job(): - cmd = [ - "litani", "add-job", - "--command", "cbmc-starter-kit-print-tool-versions .", - "--description", "printing out tool versions", - "--phony-outputs", str(uuid.uuid4()), - "--pipeline-name", "print_tool_versions", - "--ci-stage", "report", - "--tags", "front-page-text", - ] - proc = subprocess.run(cmd) - if proc.returncode: - logging.critical("Could not add tool version printing job") - sys.exit(1) - - async def main(): # pylint: disable=too-many-locals args = get_args() set_up_logging(args.verbose) @@ -419,8 +407,6 @@ async def main(): # pylint: disable=too-many-locals await proof_queue.join() - add_tool_version_job() - print_counter(counter) print("", file=sys.stderr) From 0e67b12011e846d9b941429cd8366dad744e3cdc Mon Sep 17 00:00:00 2001 From: Tony Josi Date: Tue, 18 Jul 2023 15:04:26 +0530 Subject: [PATCH 4/8] Handle upgrade header (#159) * handle upgrade header, update unit tests * update review comments * fix review comments --- source/core_http_client.c | 11 ++++++-- test/unit-test/core_http_send_utest.c | 39 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/source/core_http_client.c b/source/core_http_client.c index 63836b53..666e1f43 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -1050,6 +1050,7 @@ static HTTPStatus_t processLlhttpError( const llhttp_t * pHttpParser ) switch( llhttp_get_errno( pHttpParser ) ) { case HPE_OK: + case HPE_PAUSED_UPGRADE: /* There were no errors. */ break; @@ -1158,6 +1159,7 @@ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, { HTTPStatus_t returnStatus; const char * parsingStartLoc = NULL; + llhttp_errno_t eReturn; assert( pParsingContext != NULL ); assert( pResponse != NULL ); @@ -1202,8 +1204,13 @@ static HTTPStatus_t parseHttpResponse( HTTPParsingContext_t * pParsingContext, /* This will begin the parsing. Each of the callbacks set in * parserSettings will be invoked as parts of the HTTP response are - * reached. The return code is parsed in #processLlhttpError so is not needed. */ - ( void ) llhttp_execute( &( pParsingContext->llhttpParser ), parsingStartLoc, parseLen ); + * reached. The return code is parsed in #processLlhttpError. */ + eReturn = llhttp_execute( &( pParsingContext->llhttpParser ), parsingStartLoc, parseLen ); + + if( eReturn == HPE_PAUSED_UPGRADE ) + { + llhttp_resume_after_upgrade( &( pParsingContext->llhttpParser ) ); + } /* The next location to parse will always be after what has already * been parsed. */ diff --git a/test/unit-test/core_http_send_utest.c b/test/unit-test/core_http_send_utest.c index 4e5174f6..28197e9c 100644 --- a/test/unit-test/core_http_send_utest.c +++ b/test/unit-test/core_http_send_utest.c @@ -561,6 +561,25 @@ static llhttp_errno_t llhttp_execute_whole_response( llhttp_t * pParser, return HPE_OK; } +/* Mocked llhttp_execute callback that expects upgrade header in HTTP response. */ +static llhttp_errno_t llhttp_execute_paused_upgrade( llhttp_t * pParser, + const char * pData, + size_t len, + int cmock_num_calls ) +{ + ( void ) cmock_num_calls; + llhttp_errno_t eReturn = HPE_OK; + + if( httpParserExecuteCallCount == 0 ) + { + eReturn = HPE_PAUSED_UPGRADE; + } + + llhttp_execute_whole_response( pParser, pData, len, 0 ); + + return eReturn; +} + /* Mocked llhttp_execute callback that will be called the first time on the * response message up to the middle of the first header field, then the second * time on the response message from the middle of the first header field to the @@ -1374,6 +1393,26 @@ void test_HTTPClient_Send_less_bytes_request_body( void ) /*-----------------------------------------------------------*/ +/* Test upgrade header in HTTP response. */ +void test_HTTPClient_Send_paused_upgrade( void ) +{ + HTTPStatus_t returnStatus = HTTPSuccess; + + llhttp_execute_Stub( llhttp_execute_paused_upgrade ); + llhttp_resume_after_upgrade_ExpectAnyArgs(); + + returnStatus = HTTPClient_Send( &transportInterface, + &requestHeaders, + NULL, + 0, + &response, + 0 ); + + TEST_ASSERT_EQUAL( HTTPSuccess, returnStatus ); +} + +/*-----------------------------------------------------------*/ + /* Test when a network error is returned when receiving the response. */ void test_HTTPClient_Send_network_error_response( void ) { From b539e7ab2360efde3c6361f4c2bfcc065b22d087 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Tue, 5 Sep 2023 17:55:13 -0400 Subject: [PATCH 5/8] CI-CD Updates (#162) * Use new version of CI-CD Actions, checkout@v3 instead of checkout@v2 on all jobs * Use cSpell spell check, and use ubuntu-20.04 for formatting check * Add in bot formatting action --- .github/.cSpellWords.txt | 28 +++ .github/workflows/ci.yml | 82 ++++-- .github/workflows/formatting.yml | 23 ++ .github/workflows/release.yml | 4 +- MISRA.md | 2 +- README.md | 94 ++++--- cspell.config.yaml | 31 +++ lexicon.txt | 288 ---------------------- manifest.yml | 17 +- source/core_http_client.c | 2 +- source/include/core_http_client_private.h | 2 +- test/CMakeLists.txt | 2 +- 12 files changed, 223 insertions(+), 352 deletions(-) create mode 100644 .github/.cSpellWords.txt create mode 100644 .github/workflows/formatting.yml create mode 100644 cspell.config.yaml delete mode 100644 lexicon.txt diff --git a/.github/.cSpellWords.txt b/.github/.cSpellWords.txt new file mode 100644 index 00000000..1a4c8cb9 --- /dev/null +++ b/.github/.cSpellWords.txt @@ -0,0 +1,28 @@ +CBMC +CBOR +CMOCK +CMock +Cmock +Coverity +DCMOCK +DNDEBUG +DUNITY +Doesnt +LLHTTP +MISRA +MQTT +Misra +Wunused +abcdefghijk +abcdefghijklmnopqrstuvwxyzabcdefghijklmnopq +cbmc +cbor +cmock +coverity +ctest +lcov +llhttp +lmnopqrstuvw +misra +sinclude +utest diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2d51615..4590f56b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Clone This Repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Build run: | sudo apt-get install -y lcov @@ -21,11 +21,10 @@ jobs: -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -DNDEBUG' make -C build/ all - - name: Test - run: | - cd build/ - ctest -E system --output-on-failure - cd .. + + - name: Run CTests + run: ctest --test-dir build -E system --output-on-failure + - name: Run Coverage run: | make -C build/ coverage @@ -35,49 +34,54 @@ jobs: - name: Check Coverage uses: FreeRTOS/CI-CD-Github-Actions/coverage-cop@main with: - path: ./build/coverage.info + coverage-file: ./build/coverage.info + complexity: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Check complexity uses: FreeRTOS/CI-CD-Github-Actions/complexity@main with: path: ./ + doxygen: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Run doxygen build uses: FreeRTOS/CI-CD-Github-Actions/doxygen@main with: path: ./ + spell-check: runs-on: ubuntu-latest steps: - name: Clone This Repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Run spellings check uses: FreeRTOS/CI-CD-Github-Actions/spellings@main with: path: ./ + formatting: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Check formatting uses: FreeRTOS/CI-CD-Github-Actions/formatting@main with: path: ./ + ssot-check: runs-on: ubuntu-latest steps: - name: Checkout this repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: path: current - name: Checkout coreMQTT - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: ref: main repository: FreeRTOS/coreMQTT @@ -93,12 +97,13 @@ jobs: else exit 0 fi + git-secrets: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Checkout awslabs/git-secrets - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: repository: awslabs/git-secrets ref: master @@ -109,22 +114,57 @@ jobs: run: | git-secrets --register-aws git-secrets --scan + memory_statistics: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: - submodules: 'recursive' + submodules: "recursive" - name: Install Python3 - uses: actions/setup-python@v2 + uses: actions/setup-python@v3 with: - python-version: '3.11.0' + python-version: "3.11.0" - name: Measure sizes uses: FreeRTOS/CI-CD-Github-Actions/memory_statistics@main with: - config: .github/memory_statistics_config.json - check_against: docs/doxygen/include/size_table.md + config: .github/memory_statistics_config.json + check_against: docs/doxygen/include/size_table.md + + link-verifier: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Check Links + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + uses: FreeRTOS/CI-CD-Github-Actions/link-verifier@main + with: + path: ./ + + verify-manifest: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + submodules: true + fetch-depth: 0 + + # At time of writing the gitmodules are set not to pull + # Even when using fetch submodules. Need to run this command + # To force it to grab them. + - name: Perform Recursive Clone + shell: bash + run: git submodule update --checkout --init --recursive + + - name: Run manifest verifier + uses: FreeRTOS/CI-CD-GitHub-Actions/manifest-verifier@main + with: + path: ./ + fail-on-incorrect-version: true + proof_ci: + if: ${{ github.event.pull_request }} runs-on: cbmc_ubuntu-latest_64-core steps: - name: Set up CBMC runner diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml new file mode 100644 index 00000000..8257adda --- /dev/null +++ b/.github/workflows/formatting.yml @@ -0,0 +1,23 @@ +name: Format Pull Request Files + +on: + issue_comment: + types: [created] + +env: + bashPass: \033[32;1mPASSED - + bashInfo: \033[33;1mINFO - + bashFail: \033[31;1mFAILED - + bashEnd: \033[0m + +jobs: + Formatting: + name: Run Formatting Check + if: ${{ github.event.issue.pull_request }} && + ( ( github.event.comment.body == '/bot run uncrustify' ) || + ( github.event.comment.body == '/bot run formatting' ) ) + runs-on: ubuntu-20.04 + steps: + - name: Apply Formatting Fix + uses: FreeRTOS/CI-CD-Github-Actions/formatting-bot@main + id: check-formatting diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 43c033c4..c1802545 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: ref: ${{ github.event.inputs.commit_id }} - name: Configure git identity @@ -53,7 +53,7 @@ jobs: - name: Install ZIP tools run: sudo apt-get install zip unzip - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: ref: ${{ github.event.inputs.commit_id }} path: coreHTTP diff --git a/MISRA.md b/MISRA.md index 296b961f..f2b350de 100644 --- a/MISRA.md +++ b/MISRA.md @@ -92,4 +92,4 @@ _Ref 21.13.1_ - MISRA Rule 21.13 flags any value passed into a ctype.h function that isn't cast as an unsigned char. Thorough testing by use of our CBMC proofs shows that adding the cast to ( unsigned char ) inside of the toupper() call has potential to lead - to errors. Due to this we supress this warning for our use case. + to errors. Due to this we suppress this warning for our use case. diff --git a/README.md b/README.md index a950f89f..1dce2004 100644 --- a/README.md +++ b/README.md @@ -1,31 +1,45 @@ # coreHTTP Client Library +**[API Documentation Pages for current and previous releases of this library can be found here](https://freertos.github.io/coreHTTP/)** + + This repository contains a C language HTTP client library designed for embedded platforms. It has no dependencies on any additional libraries other than the -standard C library, [llhttp](https://github.com/nodejs/llhttp), and -a customer-implemented transport interface. This library is distributed under -the [MIT Open Source License](LICENSE). +standard C library, [llhttp](https://github.com/nodejs/llhttp), and a +customer-implemented transport interface. This library is distributed under the +[MIT Open Source License](LICENSE). This library has gone through code quality checks including verification that no -function has a [GNU Complexity](https://www.gnu.org/software/complexity/manual/complexity.html) +function has a +[GNU Complexity](https://www.gnu.org/software/complexity/manual/complexity.html) score over 8. This library has also undergone both static code analysis from [Coverity static analysis](https://scan.coverity.com/), and validation of memory safety and data structure invariance through the [CBMC automated reasoning tool](https://www.cprover.org/cbmc/). -See memory requirements for this library [here](./docs/doxygen/include/size_table.md). +See memory requirements for this library +[here](./docs/doxygen/include/size_table.md). -**coreHTTP v3.0.0 [source code](https://github.com/FreeRTOS/coreHTTP/tree/v3.0.0/source) is part of the [FreeRTOS 202210.00 LTS](https://github.com/FreeRTOS/FreeRTOS-LTS/tree/202210.00-LTS) release.** +**coreHTTP v3.0.0 +[source code](https://github.com/FreeRTOS/coreHTTP/tree/v3.0.0/source) is part +of the +[FreeRTOS 202210.00 LTS](https://github.com/FreeRTOS/FreeRTOS-LTS/tree/202210.00-LTS) +release.** -**coreHTTP v2.0.0 [source code](https://github.com/FreeRTOS/coreHTTP/tree/v2.0.0/source) is part of the [FreeRTOS 202012.00 LTS](https://github.com/FreeRTOS/FreeRTOS-LTS/tree/202012.00-LTS) release.** +**coreHTTP v2.0.0 +[source code](https://github.com/FreeRTOS/coreHTTP/tree/v2.0.0/source) is part +of the +[FreeRTOS 202012.00 LTS](https://github.com/FreeRTOS/FreeRTOS-LTS/tree/202012.00-LTS) +release.** ## coreHTTP Config File The HTTP client library exposes configuration macros that are required for building the library. A list of all the configurations and their default values -are defined in [core_http_config_defaults.h](source/include/core_http_config_defaults.h). -To provide custom values for the configuration macros, a custom config file -named `core_http_config.h` can be provided by the user application to the library. +are defined in +[core_http_config_defaults.h](source/include/core_http_config_defaults.h). To +provide custom values for the configuration macros, a custom config file named +`core_http_config.h` can be provided by the user application to the library. By default, a `core_http_config.h` custom config is required to build the library. To disable this requirement and build the library with default @@ -33,11 +47,11 @@ configuration values, provide `HTTP_DO_NOT_USE_CUSTOM_CONFIG` as a compile time preprocessor macro. **The HTTP client library can be built by either**: -* Defining a `core_http_config.h` file in the application, and adding it to the -include directories for the library build. -**OR** -* Defining the `HTTP_DO_NOT_USE_CUSTOM_CONFIG` preprocessor macro for the -library build. + +- Defining a `core_http_config.h` file in the application, and adding it to the + include directories for the library build. **OR** +- Defining the `HTTP_DO_NOT_USE_CUSTOM_CONFIG` preprocessor macro for the + library build. ## Building the Library @@ -58,19 +72,20 @@ file, refer to the `coverity_analysis` library target in ### Platform Prerequisites - For running unit tests, the following are required: - - **C90 compiler** like gcc - - **CMake 3.13.0 or later** - - **Ruby 2.0.0 or later** is required for this repository's - [CMock test framework](https://github.com/ThrowTheSwitch/CMock). + - **C90 compiler** like gcc + - **CMake 3.13.0 or later** + - **Ruby 2.0.0 or later** is required for this repository's + [CMock test framework](https://github.com/ThrowTheSwitch/CMock). - For running the coverage target, the following are required: - - **gcov** - - **lcov** + - **gcov** + - **lcov** ### Steps to build **Unit Tests** 1. Go to the root directory of this repository. -1. Run the *cmake* command: `cmake -S test -B build -DBUILD_CLONE_SUBMODULES=ON ` +1. Run the _cmake_ command: + `cmake -S test -B build -DBUILD_CLONE_SUBMODULES=ON ` 1. Run this command to build the library and unit tests: `make -C build all` @@ -80,33 +95,43 @@ file, refer to the `coverity_analysis` library target in ## CBMC - To learn more about CBMC and proofs specifically, review the training material [here](https://model-checking.github.io/cbmc-training). +To learn more about CBMC and proofs specifically, review the training material +[here](https://model-checking.github.io/cbmc-training). The `test/cbmc/proofs` directory contains CBMC proofs. -In order to run these proofs you will need to install CBMC and other tools by following the instructions [here](https://model-checking.github.io/cbmc-training/installation.html). +In order to run these proofs you will need to install CBMC and other tools by +following the instructions +[here](https://model-checking.github.io/cbmc-training/installation.html). ## Reference examples -The AWS IoT Device SDK for Embedded C repository contains demos of using the HTTP client -library [here](https://github.com/aws/aws-iot-device-sdk-embedded-C/tree/main/demos/http) -on a POSIX platform. These can be used as reference examples for the library API. +The AWS IoT Device SDK for Embedded C repository contains demos of using the +HTTP client library +[here](https://github.com/aws/aws-iot-device-sdk-embedded-C/tree/main/demos/http) +on a POSIX platform. These can be used as reference examples for the library +API. ## Documentation ### Existing Documentation -For pre-generated documentation, please see the documentation linked in the locations below: -| Location | -| :-: | +For pre-generated documentation, please see the documentation linked in the +locations below: + +| Location | +| :------------------------------------------------------------------------------------------------------------------: | | [AWS IoT Device SDK for Embedded C](https://github.com/aws/aws-iot-device-sdk-embedded-C#releases-and-documentation) | -| [FreeRTOS.org](https://freertos.org/Documentation/api-ref/coreHTTP/docs/doxygen/output/html/index.html) | +| [FreeRTOS.org](https://freertos.org/Documentation/api-ref/coreHTTP/docs/doxygen/output/html/index.html) | -Note that the latest included version of coreHTTP may differ across repositories. +Note that the latest included version of coreHTTP may differ across +repositories. ### Generating Documentation + The Doxygen references were created using Doxygen version 1.9.2. To generate the -Doxygen pages, please run the following command from the root of this repository: +Doxygen pages, please run the following command from the root of this +repository: ```shell doxygen docs/doxygen/config.doxyfile @@ -114,4 +139,5 @@ doxygen docs/doxygen/config.doxyfile ## Contributing -See [CONTRIBUTING.md](./.github/CONTRIBUTING.md) for information on contributing. +See [CONTRIBUTING.md](./.github/CONTRIBUTING.md) for information on +contributing. diff --git a/cspell.config.yaml b/cspell.config.yaml new file mode 100644 index 00000000..911ce1d8 --- /dev/null +++ b/cspell.config.yaml @@ -0,0 +1,31 @@ +--- +$schema: https://raw.githubusercontent.com/streetsidesoftware/cspell/main/cspell.schema.json +version: '0.2' +# Allows things like stringLength +allowCompoundWords: true + +# Read files not to spell check from the git ignore +useGitignore: true + +# Language settings for C +languageSettings: + - caseSensitive: false + enabled: true + languageId: c + locale: "*" + +# Add a dictionary, and the path to the word list +dictionaryDefinitions: + - name: freertos-words + path: '.github/.cSpellWords.txt' + addWords: true + +dictionaries: + - freertos-words + +# Paths and files to ignore +ignorePaths: + - 'dependency' + - 'docs' + - 'ThirdParty' + - 'History.txt' diff --git a/lexicon.txt b/lexicon.txt deleted file mode 100644 index a8aa0a01..00000000 --- a/lexicon.txt +++ /dev/null @@ -1,288 +0,0 @@ -absolutevalue -addheader -addrangeheader -addrangeheaders -addrangerequest -addtogroup -aggregator -api -apis -ascii -aws -bool -br -bufferlen -bufferlength -bytesreceived -bytesremaining -bytessent -bytestorecv -bytestosend -calltlsrecvfunc -cb -cbmc -chk -chunked -colspan -com -cond -config -configpagestyle -const -contentlength -convertint -copybrief -copydoc -corehttp -coverity -cprover -css -datalen -datelen -defgroup -doesn -doxygen -endcode -endcond -endif -enums -eof -errno -expectedheader -fieldfound -fieldlen -fieldlentoreturn -fieldloc -findheadercontext -findheaderfieldparsercallback -findheaderinresponse -findheaderonheadercompletecallback -findheadervalueparsercallback -firstpartbytes -freertos -gcc -getfinalresponsestatus -gettime -gettimestampms -github -headercount -headerslen -hostlen -hpe -html -http -httpclient -httpheadernotfound -httpheaderstrncpy -httpinsufficientmemory -httpinvalidparameter -httpinvalidresponse -httplibrarystatus -httpnetworkerror -httpnoresponse -httpparseonstatusfieldcallback -httpparser -httpparserinternalerror -httpparseronbodycallback -httpparseronheaderfieldcallback -httpparseronheaderscompletecallback -httpparseronheadervaluecallback -httpparseronmessagebegincallback -httpparseronmessagecompletecallback -httpparseronstatuscallback -httpparserxxxxcallback -httpparserxxxxcallbacks -httpparsingcontext -httpparsingstate -httppartialresponse -httprequestheaders -httprequestinfo -httpresponse -https -httpsecurityalertextraneousresponsedata -httpsecurityalertinvalidcontentlength -httpsecurityalertinvalidcharacter -httpsecurityalertinvalidchunkheader -httpsecurityalertinvalidprotocolversion -httpsecurityalertinvalidstatuscode -httpstatus -httpsuccess -ietf -ifndef -inbetween -inc -ingroup -init -initializerequestheaders -int -iot -ioveccount -ip -isfield -isheaderresponse -isheadresponse -iso -lastheaderfieldlen -lastheadervaluelen -latin -len -linux -llhttp -llhttpparser -llhttpsettings -logdebug -logerror -loginfo -logwarn -mainpage -malloc -md -memcpy -memmove -methodlen -min -misra -mit -mqtt -msg -mynetworkrecvimplementation -mynetworksendimplementation -myplatformnetworkcontext -myplatformtransportreceive -myplatformtransportsend -mytcpsocketcontext -mytlscontext -networkcontext -nodejs -noninfringement -ok -onbody -onheadercallback -onheaderfield -onheaderscomplete -onheadervalue -onmessagebegin -onmessagecomplete -onstatus -ored -org -os -param -parsehttpresponse -parselen -parsersettings -parsingstate -pathlen -pbuffer -pbuffercur -pbytesreceived -pcontext -pdata -pdateloc -pdest -pfield -pfieldloc -pfindheadercontext -pheaderparsingcallback -pheaders -phost -phttpparser -phttpparsingcontext -piovec -plaintext -plastheaderfield -plastheadervalue -ploc -psrc -pmethod -pname -pnetworkcontext -pnetworkdata -pnext -pnextwriteloc -png -posix -pparsingcontext -pparsingstate -ppath -pre -prefill -prequestbodybuf -prequestheaders -prequestinfo -presponse -processllhttperror -ptransport -ptransportinterface -pvalue -pvaluelen -pvalueloc -rangeend -rangestart -rangestartorlastnbytes -readheader -receivehttpdata -recv -recvcurrentcall -recvstopcall -recvtimeoutcall -reponse -reqbodybuflen -reqbodylen -reqflags -requestbody -requestheaderbuffer -requestheaders -requestinfo -respflags -responsebufferlen -rfc -rm -sdk -senderrorcall -sendflags -sendhttpbody -sendhttpheaders -sendpartialcall -sensitivity -sizeof -snprintf -spdx -statuscode -strchr -strerror -strncpy -struct -structs -sublicense -tcp -tcpsocketcontext -td -tls -tlscontext -tlsrecv -tlsrecvcount -tlssend -toascii -toolchain -totalreceived -tr -transportcallback -transportinterface -transportpage -transportrecv -transportsectionimplementation -transportsectionoverview -transportsend -transportstatus -transportstruct -tx -txt -uint -uri -url -valuefound -valuelen -valueloc -writev -xxxx diff --git a/manifest.yml b/manifest.yml index 580eae02..3a0c6ce2 100644 --- a/manifest.yml +++ b/manifest.yml @@ -1,11 +1,22 @@ -name : "coreHTTP" +name: "coreHTTP" version: "v3.0.0" -description: "Client implementation of the HTTP/1.1 specification for embedded devices." +description: + "Client implementation of the HTTP/1.1 specification for embedded devices." license: "MIT" + dependencies: - - name : "llhttp" + - name: "llhttp" version: "release/v6.0.5" license: "MIT" repository: type: "git" url: "https://github.com/nodejs/llhttp.git" + path: source/dependency/3rdparty/llhttp + + - name: "CMock" + version: v2.5.2 + license: "MIT" + repository: + type: "git" + url: "https://github.com/ThrowTheSwitch/CMock.git" + path: test/unit-test/CMock diff --git a/source/core_http_client.c b/source/core_http_client.c index 666e1f43..2ea4d04d 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -574,7 +574,7 @@ static int8_t caseInsensitiveStringCmp( const char * str1, size_t n ) { size_t i = 0U; - /* Inclusion of inbetween variables for MISRA rule 13.2 compliance */ + /* Inclusion of temporary variables for MISRA rule 13.2 compliance */ char firstChar; char secondChar; /* Get the offset from a lowercase to capital character in a MISRA compliant way */ diff --git a/source/include/core_http_client_private.h b/source/include/core_http_client_private.h index b2c83de4..d53e5d61 100644 --- a/source/include/core_http_client_private.h +++ b/source/include/core_http_client_private.h @@ -204,7 +204,7 @@ typedef enum HTTPParsingState_t { HTTP_PARSING_NONE = 0, /**< The parser has not started reading any response. */ - HTTP_PARSING_INCOMPLETE, /**< The parser found a partial reponse. */ + HTTP_PARSING_INCOMPLETE, /**< The parser found a partial response. */ HTTP_PARSING_COMPLETE /**< The parser found the entire response. */ } HTTPParsingState_t; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6d27d46f..4710c701 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -49,7 +49,7 @@ target_include_directories( coverity_analysis PUBLIC ${HTTP_INCLUDE_PUBLIC_DIRS} target_compile_options(coverity_analysis PUBLIC -DNDEBUG ) # ===================== Clone needed third-party libraries ====================== -# Define an llhttp paser resource path. +# Define an llhttp parser resource path. set( LLHTTP_DIR ${MODULE_ROOT_DIR}/source/dependency/3rdparty/llhttp CACHE INTERNAL "llhttp library source directory." ) include( llhttp_build.cmake ) From 76e516c9116a9a04b6f8a144db526e0471cb687b Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 5 Oct 2023 09:39:07 -0700 Subject: [PATCH 6/8] Apply formatting bot fix (#163) --- .github/workflows/formatting.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index 8257adda..04786bad 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -13,9 +13,9 @@ env: jobs: Formatting: name: Run Formatting Check - if: ${{ github.event.issue.pull_request }} && + if: ${{ github.event.issue.pull_request && ( ( github.event.comment.body == '/bot run uncrustify' ) || - ( github.event.comment.body == '/bot run formatting' ) ) + ( github.event.comment.body == '/bot run formatting' ) ) }} runs-on: ubuntu-20.04 steps: - name: Apply Formatting Fix From 15213f47ce264caaa39aafda0b73c664f17cf2c7 Mon Sep 17 00:00:00 2001 From: Giuseppe Penone Date: Wed, 25 Oct 2023 22:52:15 +0100 Subject: [PATCH 7/8] [support presigned URL] support http request without HTTP_USER_AGENT_FIELD (#164) * Added flag HTTP_REQUEST_NO_USER_AGENT_FLAG to skip adding HTTP_USER_AGENT_FIELD needed for presigned URL * Uncrustify: triggered by comment * fixed doxygen comment * Update size_table.md --------- Co-authored-by: Giuseppe Penone Co-authored-by: GitHub Action Co-authored-by: Soren Ptak --- docs/doxygen/include/size_table.md | 4 ++-- source/core_http_client.c | 15 +++++++++------ source/include/core_http_client.h | 13 ++++++++++++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/docs/doxygen/include/size_table.md b/docs/doxygen/include/size_table.md index 64dccdd3..fed45453 100644 --- a/docs/doxygen/include/size_table.md +++ b/docs/doxygen/include/size_table.md @@ -9,7 +9,7 @@ core_http_client.c -
3.1K
+
3.2K
2.5K
@@ -29,7 +29,7 @@ Total estimates -
23.9K
 +
24.0K
20.7K
 diff --git a/source/core_http_client.c b/source/core_http_client.c index 2ea4d04d..f5693eac 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -1632,12 +1632,15 @@ HTTPStatus_t HTTPClient_InitializeRequestHeaders( HTTPRequestHeaders_t * pReques if( returnStatus == HTTPSuccess ) { - /* Write "User-Agent: ". */ - returnStatus = addHeader( pRequestHeaders, - HTTP_USER_AGENT_FIELD, - HTTP_USER_AGENT_FIELD_LEN, - HTTP_USER_AGENT_VALUE, - HTTP_USER_AGENT_VALUE_LEN ); + if( ( HTTP_REQUEST_NO_USER_AGENT_FLAG & pRequestInfo->reqFlags ) == 0U ) + { + /* Write "User-Agent: ". */ + returnStatus = addHeader( pRequestHeaders, + HTTP_USER_AGENT_FIELD, + HTTP_USER_AGENT_FIELD_LEN, + HTTP_USER_AGENT_VALUE, + HTTP_USER_AGENT_VALUE_LEN ); + } } if( returnStatus == HTTPSuccess ) diff --git a/source/include/core_http_client.h b/source/include/core_http_client.h index 3425f7e0..96f2d1bd 100644 --- a/source/include/core_http_client.h +++ b/source/include/core_http_client.h @@ -111,7 +111,18 @@ * * This flag is valid only for #HTTPRequestInfo_t reqFlags parameter. */ -#define HTTP_REQUEST_KEEP_ALIVE_FLAG 0x1U +#define HTTP_REQUEST_KEEP_ALIVE_FLAG 0x1U + +/** + * @ingroup http_request_flags + * @brief Set this flag to skip the User-Agent in the request headers. + * + * Setting this will cause the "User-Agent: " to be omitted in the + * request headers. + * + * This flag is valid only for #HTTPRequestInfo_t reqFlags parameter. + */ +#define HTTP_REQUEST_NO_USER_AGENT_FLAG 0x2U /** * @defgroup http_response_flags HTTPResponse_t Flags From 668db4b3faf92a9dc96c2e3c62c716d649be0071 Mon Sep 17 00:00:00 2001 From: V <36897290+cryi@users.noreply.github.com> Date: Thu, 9 Nov 2023 00:01:47 +0100 Subject: [PATCH 8/8] Expose Internal Functions for Enhanced Usability (#167) This exposes the internal functions sendHttpHeaders, sendHttpData, and receiveAndParseHttpResponse. Exposing these functions aims to provide developers with more control over the HTTP communication process, enabling the support for chunked body reads and writes, streaming, and other advanced HTTP functionalities which are essential for building more sophisticated applications atop coreHTTP. --- docs/doxygen/include/size_table.md | 4 +- source/core_http_client.c | 120 ++++++++-------------- source/include/core_http_client.h | 91 ++++++++++++++++ test/cbmc/proofs/HTTPClient_Send/Makefile | 4 +- 4 files changed, 139 insertions(+), 80 deletions(-) diff --git a/docs/doxygen/include/size_table.md b/docs/doxygen/include/size_table.md index fed45453..2170e484 100644 --- a/docs/doxygen/include/size_table.md +++ b/docs/doxygen/include/size_table.md @@ -10,7 +10,7 @@ core_http_client.c
3.2K
-
2.5K
+
2.6K
api.c (llhttp) @@ -30,6 +30,6 @@ Total estimates
24.0K
 -
20.7K
 +
20.8K
 diff --git a/source/core_http_client.c b/source/core_http_client.c index f5693eac..995143ff 100644 --- a/source/core_http_client.c +++ b/source/core_http_client.c @@ -43,43 +43,18 @@ */ static uint32_t getZeroTimestampMs( void ); -/** - * @brief Send HTTP bytes over the transport send interface. - * - * @param[in] pTransport Transport interface. - * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. - * @param[in] pData HTTP request data to send. - * @param[in] dataLen HTTP request data length. - * - * @return #HTTPSuccess if successful. If there was a network error or less - * bytes than what were specified were sent, then #HTTPNetworkError is - * returned. - */ -static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, - HTTPClient_GetCurrentTimeFunc_t getTimestampMs, - const uint8_t * pData, - size_t dataLen ); -/** - * @brief Send the HTTP headers over the transport send interface. - * - * @param[in] pTransport Transport interface. - * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. - * @param[in] pRequestHeaders Request headers to send, it includes the buffer - * and length. - * @param[in] reqBodyLen The length of the request body to be sent. This is - * used to generated a Content-Length header. - * @param[in] sendFlags Application provided flags to #HTTPClient_Send. - * - * @return #HTTPSuccess if successful. If there was a network error or less - * bytes than what were specified were sent, then #HTTPNetworkError is - * returned. - */ -static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, - HTTPClient_GetCurrentTimeFunc_t getTimestampMs, - HTTPRequestHeaders_t * pRequestHeaders, - size_t reqBodyLen, - uint32_t sendFlags ); +HTTPStatus_t HTTPClient_SendHttpData( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + const uint8_t * pData, + size_t dataLen ); + + +HTTPStatus_t HTTPClient_SendHttpHeaders( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + HTTPRequestHeaders_t * pRequestHeaders, + size_t reqBodyLen, + uint32_t sendFlags ); /** * @brief Adds the Content-Length header field and value to the @@ -187,20 +162,10 @@ static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, size_t totalReceived, size_t responseBufferLen ); -/** - * @brief Receive the HTTP response from the network and parse it. - * - * @param[in] pTransport Transport interface. - * @param[in] pResponse Response message to receive data from the network. - * @param[in] pRequestHeaders Request headers for the corresponding HTTP request. - * - * @return Returns #HTTPSuccess if successful. #HTTPNetworkError for a transport - * receive error. Please see #parseHttpResponse and #getFinalResponseStatus for - * other statuses returned. - */ -static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pTransport, - HTTPResponse_t * pResponse, - const HTTPRequestHeaders_t * pRequestHeaders ); + +HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t * pTransport, + HTTPResponse_t * pResponse, + const HTTPRequestHeaders_t * pRequestHeaders ); /** * @brief Send the HTTP request over the network. @@ -212,7 +177,7 @@ static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pT * @param[in] reqBodyBufLen Length of the request body buffer. * @param[in] sendFlags Application provided flags to #HTTPClient_Send. * - * @return Returns #HTTPSuccess if successful. Please see #sendHttpHeaders and + * @return Returns #HTTPSuccess if successful. Please see #HTTPClient_SendHttpHeaders and * #sendHttpBody for other statuses returned. */ static HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, @@ -833,6 +798,9 @@ static int httpParserOnHeadersCompleteCallback( llhttp_t * pHttpParser ) assert( pResponse != NULL ); assert( pParsingContext->pBufferCur != NULL ); + /* Flag indicating that the headers have been completely signed - useful for libraries built on top of corehttp. */ + pResponse->areHeadersComplete = 1; + /* The current location to parse was updated in previous callbacks and MUST * always be within the response buffer. */ assert( pParsingContext->pBufferCur >= ( const char * ) ( pResponse->pBuffer ) ); @@ -1796,10 +1764,10 @@ HTTPStatus_t HTTPClient_AddRangeHeader( HTTPRequestHeaders_t * pRequestHeaders, /*-----------------------------------------------------------*/ -static HTTPStatus_t sendHttpData( const TransportInterface_t * pTransport, - HTTPClient_GetCurrentTimeFunc_t getTimestampMs, - const uint8_t * pData, - size_t dataLen ) +HTTPStatus_t HTTPClient_SendHttpData( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + const uint8_t * pData, + size_t dataLen ) { HTTPStatus_t returnStatus = HTTPSuccess; const uint8_t * pIndex = pData; @@ -1908,11 +1876,11 @@ static HTTPStatus_t addContentLengthHeader( HTTPRequestHeaders_t * pRequestHeade /*-----------------------------------------------------------*/ -static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, - HTTPClient_GetCurrentTimeFunc_t getTimestampMs, - HTTPRequestHeaders_t * pRequestHeaders, - size_t reqBodyLen, - uint32_t sendFlags ) +HTTPStatus_t HTTPClient_SendHttpHeaders( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + HTTPRequestHeaders_t * pRequestHeaders, + size_t reqBodyLen, + uint32_t sendFlags ) { HTTPStatus_t returnStatus = HTTPSuccess; uint8_t shouldSendContentLength = 0U; @@ -1935,10 +1903,10 @@ static HTTPStatus_t sendHttpHeaders( const TransportInterface_t * pTransport, { LogDebug( ( "Sending HTTP request headers: HeaderBytes=%lu", ( unsigned long ) ( pRequestHeaders->headersLen ) ) ); - returnStatus = sendHttpData( pTransport, - getTimestampMs, - pRequestHeaders->pBuffer, - pRequestHeaders->headersLen ); + returnStatus = HTTPClient_SendHttpData( pTransport, + getTimestampMs, + pRequestHeaders->pBuffer, + pRequestHeaders->headersLen ); } return returnStatus; @@ -1960,7 +1928,7 @@ static HTTPStatus_t sendHttpBody( const TransportInterface_t * pTransport, /* Send the request body. */ LogDebug( ( "Sending the HTTP request body: BodyBytes=%lu", ( unsigned long ) reqBodyBufLen ) ); - returnStatus = sendHttpData( pTransport, getTimestampMs, pRequestBodyBuf, reqBodyBufLen ); + returnStatus = HTTPClient_SendHttpData( pTransport, getTimestampMs, pRequestBodyBuf, reqBodyBufLen ); return returnStatus; } @@ -2014,9 +1982,9 @@ static HTTPStatus_t getFinalResponseStatus( HTTPParsingState_t parsingState, /*-----------------------------------------------------------*/ -static HTTPStatus_t receiveAndParseHttpResponse( const TransportInterface_t * pTransport, - HTTPResponse_t * pResponse, - const HTTPRequestHeaders_t * pRequestHeaders ) +HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t * pTransport, + HTTPResponse_t * pResponse, + const HTTPRequestHeaders_t * pRequestHeaders ) { HTTPStatus_t returnStatus = HTTPSuccess; size_t totalReceived = 0U; @@ -2149,11 +2117,11 @@ static HTTPStatus_t sendHttpRequest( const TransportInterface_t * pTransport, assert( getTimestampMs != NULL ); /* Send the headers, which are at one location in memory. */ - returnStatus = sendHttpHeaders( pTransport, - getTimestampMs, - pRequestHeaders, - reqBodyBufLen, - sendFlags ); + returnStatus = HTTPClient_SendHttpHeaders( pTransport, + getTimestampMs, + pRequestHeaders, + reqBodyBufLen, + sendFlags ); /* Send the body, which is at another location in memory. */ if( returnStatus == HTTPSuccess ) @@ -2269,9 +2237,9 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, if( returnStatus == HTTPSuccess ) { - returnStatus = receiveAndParseHttpResponse( pTransport, - pResponse, - pRequestHeaders ); + returnStatus = HTTPClient_ReceiveAndParseHttpResponse( pTransport, + pResponse, + pRequestHeaders ); } return returnStatus; diff --git a/source/include/core_http_client.h b/source/include/core_http_client.h index 96f2d1bd..8dc2a098 100644 --- a/source/include/core_http_client.h +++ b/source/include/core_http_client.h @@ -528,6 +528,13 @@ typedef struct HTTPResponse */ size_t headerCount; + /** + * @brief Indicates whether the HTTP response headers have been fully received. + * + * This variable is set to 1 after all headers have been received and processed by #HTTPClient_Send. + */ + uint8_t areHeadersComplete; + /** * @brief Flags of useful headers found in the response. * @@ -730,6 +737,69 @@ HTTPStatus_t HTTPClient_AddRangeHeader( HTTPRequestHeaders_t * pRequestHeaders, int32_t rangeEnd ); /* @[declare_httpclient_addrangeheader] */ +/** + * @brief Send the request headers in @p pRequestHeaders over the transport. + * + * If #HTTP_SEND_DISABLE_CONTENT_LENGTH_FLAG is not set in parameter @p sendFlags, + * then the Content-Length to be sent to the server is automatically written to + * @p pRequestHeaders. The Content-Length will not be written when there is + * no request body. If there is not enough room in the buffer to write the + * Content-Length then #HTTPInsufficientMemory is returned. Please see + * #HTTP_MAX_CONTENT_LENGTH_HEADER_LENGTH for the maximum Content-Length header + * field and value that could be written to the buffer. + * + * The application should close the connection with the server if any of the + * following errors are returned: + * - #HTTPSecurityAlertExtraneousResponseData + * - #HTTPSecurityAlertInvalidChunkHeader + * - #HTTPSecurityAlertInvalidProtocolVersion + * - #HTTPSecurityAlertInvalidStatusCode + * - #HTTPSecurityAlertInvalidCharacter + * - #HTTPSecurityAlertInvalidContentLength + * + * + * @param[in] pTransport Transport interface, see #TransportInterface_t for + * more information. + * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. + * @param[in] pRequestHeaders Request configuration containing the buffer of headers to + * send. + * @param[in] reqBodyLen The length of the request entity in bytes. + * @param[in] sendFlags Flags which modify the behavior of this function. Please see @ref + * http_send_flags for more information. + * + * @return #HTTPSuccess if successful. If there was a network error or less + * bytes than what were specified were sent, then #HTTPNetworkError is + * returned. + * + */ +/* @[declare_httpclient_sendhttpheaders] */ +HTTPStatus_t HTTPClient_SendHttpHeaders( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + HTTPRequestHeaders_t * pRequestHeaders, + size_t reqBodyLen, + uint32_t sendFlags ); +/* @[declare_httpclient_sendhttpheaders] */ + +/** + * @brief Send the request body in @p pRequestBodyBuf over the transport. + * + * @param[in] pTransport Transport interface, see #TransportInterface_t for + * more information. + * @param[in] getTimestampMs Function to retrieve a timestamp in milliseconds. + * @param[in] pData HTTP request data to send. + * @param[in] dataLen HTTP request data length. + * + * @return #HTTPSuccess if successful. If there was a network error or less + * bytes than what were specified were sent, then #HTTPNetworkError is + * returned. + */ +/* @[declare_httpclient_sendhttpdata] */ +HTTPStatus_t HTTPClient_SendHttpData( const TransportInterface_t * pTransport, + HTTPClient_GetCurrentTimeFunc_t getTimestampMs, + const uint8_t * pData, + size_t dataLen ); +/* @[declare_httpclient_sendhttpdata] */ + /** * @brief Send the request headers in #HTTPRequestHeaders_t.pBuffer and request * body in @p pRequestBodyBuf over the transport. The response is received in @@ -832,6 +902,27 @@ HTTPStatus_t HTTPClient_Send( const TransportInterface_t * pTransport, uint32_t sendFlags ); /* @[declare_httpclient_send] */ +/** + * @brief Receive the HTTP response from the network and parse it. + * + * @param[in] pTransport Transport interface, see #TransportInterface_t for more + * information. + * @param[in] pResponse The response message and some notable response parameters will be + * returned here on success. + * @param[in] pRequestHeaders Request configuration containing the buffer of headers to + * send. + * + * @return Returns #HTTPSuccess if successful. #HTTPNetworkError for a transport + * receive error. Please see #parseHttpResponse and #getFinalResponseStatus for + * other statuses returned. + * + */ +/* @[declare_httpclient_receiveandparsehttpresponse] */ +HTTPStatus_t HTTPClient_ReceiveAndParseHttpResponse( const TransportInterface_t * pTransport, + HTTPResponse_t * pResponse, + const HTTPRequestHeaders_t * pRequestHeaders ); +/* @[declare_httpclient_receiveandparsehttpresponse] */ + /** * @brief Read a header from a buffer containing a complete HTTP response. * This will return the location of the response header value in the diff --git a/test/cbmc/proofs/HTTPClient_Send/Makefile b/test/cbmc/proofs/HTTPClient_Send/Makefile index 931729ad..064330a3 100644 --- a/test/cbmc/proofs/HTTPClient_Send/Makefile +++ b/test/cbmc/proofs/HTTPClient_Send/Makefile @@ -57,8 +57,8 @@ REMOVE_FUNCTION_BODY += __CPROVER_file_local_core_http_client_c_httpHeaderStrncp # than the total possible iterations in the int32_t to ASCII converation. UNWINDSET += __CPROVER_file_local_core_http_client_c_convertInt32ToAscii.0:11 UNWINDSET += __CPROVER_file_local_core_http_client_c_convertInt32ToAscii.1:11 -UNWINDSET += __CPROVER_file_local_core_http_client_c_receiveAndParseHttpResponse.0:10 -UNWINDSET += __CPROVER_file_local_core_http_client_c_sendHttpData.0:10 +UNWINDSET += HTTPClient_ReceiveAndParseHttpResponse.0:10 +UNWINDSET += HTTPClient_SendHttpData.0:10 # strncmp is used to find if there exists "\r\n\r\n" at the end of the header # buffer. Therefore, we need to unwind strncmp to the length of "\r\n\r\n" + 1.