#!/usr/bin/env python
#
# A library that provides a Python interface to the Telegram Bot API
# Copyright (C) 2015-2023
# Leandro Toledo de Souza <devs@python-telegram-bot.org>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Lesser Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU Lesser Public License for more details.
#
# You should have received a copy of the GNU Lesser Public License
# along with this program.  If not, see [http://www.gnu.org/licenses/].
# pylint: disable=missing-module-docstring
import asyncio
import json
from http import HTTPStatus
from ssl import SSLContext
from types import TracebackType
from typing import TYPE_CHECKING, Optional, Type

# Instead of checking for ImportError here, we do that in `updater.py`, where we import from
# this module. Doing it here would be tricky, as the classes below subclass tornado classes
import tornado.web
from tornado.httpserver import HTTPServer

from telegram import Update
from telegram._utils.logging import get_logger
from telegram.ext._extbot import ExtBot

if TYPE_CHECKING:
    from telegram import Bot

# This module is not visible to users, so we log as Updater
_LOGGER = get_logger(__name__, class_name="Updater")


class WebhookServer:
    """Thin wrapper around ``tornado.httpserver.HTTPServer``."""

    __slots__ = (
        "_http_server",
        "listen",
        "port",
        "is_running",
        "_server_lock",
        "_shutdown_lock",
    )

    def __init__(
        self, listen: str, port: int, webhook_app: "WebhookAppClass", ssl_ctx: Optional[SSLContext]
    ):
        self._http_server = HTTPServer(webhook_app, ssl_options=ssl_ctx)
        self.listen = listen
        self.port = port
        self.is_running = False
        self._server_lock = asyncio.Lock()
        self._shutdown_lock = asyncio.Lock()

    async def serve_forever(self, ready: Optional[asyncio.Event] = None) -> None:
        async with self._server_lock:
            self._http_server.listen(self.port, address=self.listen)

            self.is_running = True
            if ready is not None:
                ready.set()

            _LOGGER.debug("Webhook Server started.")

    async def shutdown(self) -> None:
        async with self._shutdown_lock:
            if not self.is_running:
                _LOGGER.debug("Webhook Server is already shut down. Returning")
                return
            self.is_running = False
            self._http_server.stop()
            await self._http_server.close_all_connections()
            _LOGGER.debug("Webhook Server stopped")


class WebhookAppClass(tornado.web.Application):
    """Application used in the Webserver"""

    def __init__(
        self,
        webhook_path: str,
        bot: "Bot",
        update_queue: asyncio.Queue,
        secret_token: Optional[str] = None,
    ):
        self.shared_objects = {
            "bot": bot,
            "update_queue": update_queue,
            "secret_token": secret_token,
        }
        handlers = [(rf"{webhook_path}/?", TelegramHandler, self.shared_objects)]
        tornado.web.Application.__init__(self, handlers)  # type: ignore

    def log_request(self, handler: tornado.web.RequestHandler) -> None:
        """Overrides the default implementation since we have our own logging setup."""


# pylint: disable=abstract-method
class TelegramHandler(tornado.web.RequestHandler):
    """BaseHandler that processes incoming requests from Telegram"""

    __slots__ = ("bot", "update_queue", "secret_token")

    SUPPORTED_METHODS = ("POST",)  # type: ignore[assignment]

    def initialize(self, bot: "Bot", update_queue: asyncio.Queue, secret_token: str) -> None:
        """Initialize for each request - that's the interface provided by tornado"""
        # pylint: disable=attribute-defined-outside-init
        self.bot = bot
        self.update_queue = update_queue  # skipcq: PYL-W0201
        self.secret_token = secret_token  # skipcq: PYL-W0201
        if secret_token:
            _LOGGER.debug(
                "The webhook server has a secret token, expecting it in incoming requests now"
            )

    def set_default_headers(self) -> None:
        """Sets default headers"""
        self.set_header("Content-Type", 'application/json; charset="utf-8"')

    async def post(self) -> None:
        """Handle incoming POST request"""
        _LOGGER.debug("Webhook triggered")
        self._validate_post()

        json_string = self.request.body.decode()
        data = json.loads(json_string)
        self.set_status(HTTPStatus.OK)
        _LOGGER.debug("Webhook received data: %s", json_string)

        try:
            update = Update.de_json(data, self.bot)
        except Exception as exc:
            _LOGGER.critical(
                "Something went wrong processing the data received from Telegram. "
                "Received data was *not* processed!",
                exc_info=exc,
            )

        if update:
            _LOGGER.debug(
                "Received Update with ID %d on Webhook",
                # For some reason pylint thinks update is a general TelegramObject
                update.update_id,  # pylint: disable=no-member
            )

            # handle arbitrary callback data, if necessary
            if isinstance(self.bot, ExtBot):
                self.bot.insert_callback_data(update)

            await self.update_queue.put(update)

    def _validate_post(self) -> None:
        """Only accept requests with content type JSON"""
        ct_header = self.request.headers.get("Content-Type", None)
        if ct_header != "application/json":
            raise tornado.web.HTTPError(HTTPStatus.FORBIDDEN)
        # verifying that the secret token is the one the user set when the user set one
        if self.secret_token is not None:
            token = self.request.headers.get("X-Telegram-Bot-Api-Secret-Token")
            if not token:
                _LOGGER.debug("Request did not include the secret token")
                raise tornado.web.HTTPError(
                    HTTPStatus.FORBIDDEN, reason="Request did not include the secret token"
                )
            if token != self.secret_token:
                _LOGGER.debug("Request had the wrong secret token: %s", token)
                raise tornado.web.HTTPError(
                    HTTPStatus.FORBIDDEN, reason="Request had the wrong secret token"
                )

    def log_exception(
        self,
        typ: Optional[Type[BaseException]],
        value: Optional[BaseException],
        tb: Optional[TracebackType],
    ) -> None:
        """Override the default logging and instead use our custom logging."""
        _LOGGER.debug(
            "%s - %s",
            self.request.remote_ip,
            "Exception in TelegramHandler",
            exc_info=(typ, value, tb) if typ and value and tb else value,
        )

telegram/ext/_utils/webhookhandler.py
#!/usr/bin/env python
#
# A library that provides a Python interface to the Telegram Bot API
# Copyright (C) 2015-2023
# Leandro Toledo de Souza <devs@python-telegram-bot.org>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Lesser Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU Lesser Public License for more details.
#
# You should have received a copy of the GNU Lesser Public License
# along with this program.  If not, see [http://www.gnu.org/licenses/].
# pylint: disable=missing-module-docstring
import asyncio
import json
from http import HTTPStatus
from ssl import SSLContext
from types import TracebackType
from typing import TYPE_CHECKING, Optional, Type

# Instead of checking for ImportError here, we do that in `updater.py`, where we import from
# this module. Doing it here would be tricky, as the classes below subclass tornado classes
import tornado.web
from tornado.httpserver import HTTPServer

from telegram import Update
from telegram._utils.logging import get_logger
from telegram.ext._extbot import ExtBot

if TYPE_CHECKING:
    from telegram import Bot

# This module is not visible to users, so we log as Updater
_LOGGER = get_logger(__name__, class_name="Updater")


class WebhookServer:
    """Thin wrapper around ``tornado.httpserver.HTTPServer``."""

    __slots__ = (
        "_http_server",
        "listen",
        "port",
        "is_running",
        "_server_lock",
        "_shutdown_lock",
    )

    def __init__(
        self, listen: str, port: int, webhook_app: "WebhookAppClass", ssl_ctx: Optional[SSLContext]
    ):
        self._http_server = HTTPServer(webhook_app, ssl_options=ssl_ctx)
        self.listen = listen
        self.port = port
        self.is_running = False
        self._server_lock = asyncio.Lock()
        self._shutdown_lock = asyncio.Lock()

    async def serve_forever(self, ready: Optional[asyncio.Event] = None) -> None:
        async with self._server_lock:
            self._http_server.listen(self.port, address=self.listen)

            self.is_running = True
            if ready is not None:
                ready.set()

            _LOGGER.debug("Webhook Server started.")

    async def shutdown(self) -> None:
        async with self._shutdown_lock:
            if not self.is_running:
                _LOGGER.debug("Webhook Server is already shut down. Returning")
                return
            self.is_running = False
            self._http_server.stop()
            await self._http_server.close_all_connections()
            _LOGGER.debug("Webhook Server stopped")


class WebhookAppClass(tornado.web.Application):
    """Application used in the Webserver"""

    def __init__(
        self,
        webhook_path: str,
        bot: "Bot",
        update_queue: asyncio.Queue,
        secret_token: Optional[str] = None,
    ):
        self.shared_objects = {
            "bot": bot,
            "update_queue": update_queue,
            "secret_token": secret_token,
        }
        handlers = [(rf"{webhook_path}/?", TelegramHandler, self.shared_objects)]
        tornado.web.Application.__init__(self, handlers)  # type: ignore

    def log_request(self, handler: tornado.web.RequestHandler) -> None:
        """Overrides the default implementation since we have our own logging setup."""


# pylint: disable=abstract-method
class TelegramHandler(tornado.web.RequestHandler):
    """BaseHandler that processes incoming requests from Telegram"""

    __slots__ = ("bot", "update_queue", "secret_token")

    SUPPORTED_METHODS = ("POST",)  # type: ignore[assignment]

    def initialize(self, bot: "Bot", update_queue: asyncio.Queue, secret_token: str) -> None:
        """Initialize for each request - that's the interface provided by tornado"""
        # pylint: disable=attribute-defined-outside-init
        self.bot = bot
        self.update_queue = update_queue  # skipcq: PYL-W0201
        self.secret_token = secret_token  # skipcq: PYL-W0201
        if secret_token:
            _LOGGER.debug(
                "The webhook server has a secret token, expecting it in incoming requests now"
            )

    def set_default_headers(self) -> None:
        """Sets default headers"""
        self.set_header("Content-Type", 'application/json; charset="utf-8"')

    async def post(self) -> None:
        """Handle incoming POST request"""
        _LOGGER.debug("Webhook triggered")
        self._validate_post()

        json_string = self.request.body.decode()
        data = json.loads(json_string)
        self.set_status(HTTPStatus.OK)
        _LOGGER.debug("Webhook received data: %s", json_string)

        try:
            update = Update.de_json(data, self.bot)
        except Exception as exc:
            _LOGGER.critical(
                "Something went wrong processing the data received from Telegram. "
                "Received data was *not* processed!",
                exc_info=exc,
            )
            raise tornado.web.HTTPError(
                HTTPStatus.BAD_REQUEST, reason="Update could not be processed"
            ) from exc

        if update:
            _LOGGER.debug(
                "Received Update with ID %d on Webhook",
                # For some reason pylint thinks update is a general TelegramObject
                update.update_id,  # pylint: disable=no-member
            )

            # handle arbitrary callback data, if necessary
            if isinstance(self.bot, ExtBot):
                self.bot.insert_callback_data(update)

            await self.update_queue.put(update)

    def _validate_post(self) -> None:
        """Only accept requests with content type JSON"""
        ct_header = self.request.headers.get("Content-Type", None)
        if ct_header != "application/json":
            raise tornado.web.HTTPError(HTTPStatus.FORBIDDEN)
        # verifying that the secret token is the one the user set when the user set one
        if self.secret_token is not None:
            token = self.request.headers.get("X-Telegram-Bot-Api-Secret-Token")
            if not token:
                _LOGGER.debug("Request did not include the secret token")
                raise tornado.web.HTTPError(
                    HTTPStatus.FORBIDDEN, reason="Request did not include the secret token"
                )
            if token != self.secret_token:
                _LOGGER.debug("Request had the wrong secret token: %s", token)
                raise tornado.web.HTTPError(
                    HTTPStatus.FORBIDDEN, reason="Request had the wrong secret token"
                )

    def log_exception(
        self,
        typ: Optional[Type[BaseException]],
        value: Optional[BaseException],
        tb: Optional[TracebackType],
    ) -> None:
        """Override the default logging and instead use our custom logging."""
        _LOGGER.debug(
            "%s - %s",
            self.request.remote_ip,
            "Exception in TelegramHandler",
            exc_info=(typ, value, tb) if typ and value and tb else value,
        )

telegram/ext/_utils/webhookhandler.py
diff --git a/telegram/ext/_utils/webhookhandler.py b/telegram/ext/_utils/webhookhandler.py
index 05ad223df20..65f37ce7b76 100644
--- a/telegram/ext/_utils/webhookhandler.py
+++ b/telegram/ext/_utils/webhookhandler.py
@@ -146,6 +146,9 @@ async def post(self) -> None:
                 "Received data was *not* processed!",
                 exc_info=exc,
             )
+            raise tornado.web.HTTPError(
+                HTTPStatus.BAD_REQUEST, reason="Update could not be processed"
+            ) from exc
 
         if update:
             _LOGGER.debug(
diff --git a/tests/ext/test_updater.py b/tests/ext/test_updater.py
index f346b48bb9b..58c8804f94d 100644
--- a/tests/ext/test_updater.py
+++ b/tests/ext/test_updater.py
@@ -63,6 +63,7 @@ class TestUpdater:
     cb_handler_called = None
     offset = 0
     test_flag = False
+    response_text = "<html><title>{1}: {0}</title><body>{1}: {0}</body></html>"
 
     @pytest.fixture(autouse=True)
     def _reset(self):
@@ -732,11 +733,10 @@ async def set_webhook(*args, **kwargs):
 
             if secret_token:
                 # Returns Forbidden if no secret token is set
-                response_text = "<html><title>403: {0}</title><body>403: {0}</body></html>"
                 response = await send_webhook_message(ip, port, update.to_json(), "TOKEN")
                 assert response.status_code == HTTPStatus.FORBIDDEN
-                assert response.text == response_text.format(
-                    "Request did not include the secret token"
+                assert response.text == self.response_text.format(
+                    "Request did not include the secret token", HTTPStatus.FORBIDDEN
                 )
 
                 # Returns Forbidden if the secret token is wrong
@@ -744,7 +744,9 @@ async def set_webhook(*args, **kwargs):
                     ip, port, update.to_json(), "TOKEN", secret_token="NotTheSecretToken"
                 )
                 assert response.status_code == HTTPStatus.FORBIDDEN
-                assert response.text == response_text.format("Request had the wrong secret token")
+                assert response.text == self.response_text.format(
+                    "Request had the wrong secret token", HTTPStatus.FORBIDDEN
+                )
 
             await updater.stop()
             assert not updater.running
@@ -1071,11 +1073,15 @@ def de_json_fails(*args, **kwargs):
             # Now, we send an update to the server
             update = make_message_update("Webhook")
             with caplog.at_level(logging.CRITICAL):
-                await send_webhook_message(ip, port, update.to_json(), "TOKEN")
+                response = await send_webhook_message(ip, port, update.to_json(), "TOKEN")
 
             assert len(caplog.records) == 1
             assert caplog.records[-1].getMessage().startswith("Something went wrong processing")
             assert caplog.records[-1].name == "telegram.ext.Updater"
+            assert response.status_code == 400
+            assert response.text == self.response_text.format(
+                "Update could not be processed", HTTPStatus.BAD_REQUEST
+            )
 
             # Make sure that everything works fine again when receiving proper updates
             caplog.clear()

UnboundLocalError when submitting smth else then an Update to the webhook
https://github.com/python-telegram-bot/python-telegram-bot/blob/dd9af64a5c99c2518ef86027e008cdb8659dd1f5/telegram/ext/_utils/webhookhandler.py#L141-L148

I realized that when putting a not decodable "Update", the appropriate log is shown, but since the code continues to run, the other party receives a non descriptive 500 Internal Server Error and in the traceback an UnboundLocalError appears.

This could be easily fixed with adding `update = None` to line 149, but this means the sending party receives 200 OK while it is certainly not OK.

However, if I add a descriptive error here, bad actors could realize that they hit a telegram bot webhook and try to abuse it/spam it.  Do we want that?

If people want to protect their webhooks, they should add the secret anyway. There we however raise if the wrong/none token has been supplied, already exposing that a PTB webhook is located behind the URL...

<details>
 <summary>Traceback if you want to look at it</summary>

```
DEBUG:telegram.ext.Updater:0.0.0.0 - Exception in TelegramHandler
Traceback (most recent call last):
  File "/home/poolitzer/python-telegram-bot/venv/lib/python3.11/site-packages/tornado/web.py", line 1786, in _execute
    result = await result
             ^^^^^^^^^^^^
  File "/home/poolitzer/python-telegram-bot/telegram/ext/_utils/webhookhandler.py", line 159, in post
    if update:
       ^^^^^^
UnboundLocalError: cannot access local variable 'update' where it is not associated with a value
```

</details>

