Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use the proper Request in type hints. #9515

Merged
merged 9 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9515.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix incorrect type hints.
4 changes: 2 additions & 2 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import bcrypt
import pymacaroons

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.constants import LoginType
from synapse.api.errors import (
Expand Down Expand Up @@ -481,7 +481,7 @@ async def check_ui_auth(
sid = authdict["session"]

# Convert the URI and method to strings.
uri = request.uri.decode("utf-8")
uri = request.uri.decode("utf-8") # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of request.uri is incorrectly specified in Twisted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it isn't "incorrect", but the placeholder is a string and the value is bytes, which is annoying.

Copy link
Contributor

@ShadowJonathan ShadowJonathan Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Maybe make notes on the places where this happens, and submit that issue on twisted/twisted?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm already planning to.

method = request.method.decode("utf-8")

# If there's no session ID, create a new session.
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
import attr
from typing_extensions import NoReturn, Protocol

from twisted.web.http import Request
from twisted.web.iweb import IRequest
from twisted.web.server import Request

from synapse.api.constants import LoginType
from synapse.api.errors import Codes, NotFoundError, RedirectException, SynapseError
Expand Down
9 changes: 4 additions & 5 deletions synapse/replication/http/membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
import logging
from typing import TYPE_CHECKING, List, Optional, Tuple

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.http.servlet import parse_json_object_from_request
from synapse.http.site import SynapseRequest
from synapse.replication.http._base import ReplicationEndpoint
from synapse.types import JsonDict, Requester, UserID
from synapse.util.distributor import user_left_room
Expand Down Expand Up @@ -78,15 +79,14 @@ async def _serialize_payload( # type: ignore
}

async def _handle_request( # type: ignore
self, request: Request, room_id: str, user_id: str
self, request: SynapseRequest, room_id: str, user_id: str
) -> Tuple[int, JsonDict]:
content = parse_json_object_from_request(request)

remote_room_hosts = content["remote_room_hosts"]
event_content = content["content"]

requester = Requester.deserialize(self.store, content["requester"])

request.requester = requester

logger.info("remote_join: %s into room: %s", user_id, room_id)
Expand Down Expand Up @@ -147,15 +147,14 @@ async def _serialize_payload( # type: ignore
}

async def _handle_request( # type: ignore
self, request: Request, invite_event_id: str
self, request: SynapseRequest, invite_event_id: str
) -> Tuple[int, JsonDict]:
content = parse_json_object_from_request(request)

txn_id = content["txn_id"]
event_content = content["content"]

requester = Requester.deserialize(self.store, content["requester"])

request.requester = requester

# hopefully we're now on the master, so this won't recurse!
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/admin/media.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import logging
from typing import TYPE_CHECKING, Tuple

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError
from synapse.http.servlet import RestServlet, parse_boolean, parse_integer
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/v2_alpha/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from functools import wraps
from typing import TYPE_CHECKING, Optional, Tuple

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.constants import (
MAX_GROUP_CATEGORYID_LENGTH,
Expand Down
18 changes: 10 additions & 8 deletions synapse/rest/media/v1/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from twisted.internet.interfaces import IConsumer
from twisted.protocols.basic import FileSender
from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import Codes, SynapseError, cs_error
from synapse.http.server import finish_request, respond_with_json
Expand Down Expand Up @@ -49,18 +49,20 @@

def parse_media_id(request: Request) -> Tuple[str, str, Optional[str]]:
try:
# The type on postpath seems incorrect in Twisted 21.2.0.
postpath = request.postpath # type: List[bytes] # type: ignore
assert postpath

# This allows users to append e.g. /test.png to the URL. Useful for
# clients that parse the URL to see content type.
server_name, media_id = request.postpath[:2]

if isinstance(server_name, bytes):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always bytes so I don't think we need this if-statement.

server_name = server_name.decode("utf-8")
media_id = media_id.decode("utf8")
server_name_bytes, media_id_bytes = postpath[:2]
server_name = server_name_bytes.decode("utf-8")
media_id = media_id_bytes.decode("utf8")

file_name = None
if len(request.postpath) > 2:
if len(postpath) > 2:
try:
file_name = urllib.parse.unquote(request.postpath[-1].decode("utf-8"))
file_name = urllib.parse.unquote(postpath[-1].decode("utf-8"))
except UnicodeDecodeError:
pass
return server_name, media_id, file_name
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/config_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from typing import TYPE_CHECKING

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.http.server import DirectServeJsonResource, respond_with_json

Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/download_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import logging
from typing import TYPE_CHECKING

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.http.server import DirectServeJsonResource, set_cors_headers
from synapse.http.servlet import parse_boolean
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@

import twisted.internet.error
import twisted.web.http
from twisted.web.http import Request
from twisted.web.resource import Resource
from twisted.web.server import Request

from synapse.api.errors import (
FederationDeniedError,
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/preview_url_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import attr

from twisted.internet.error import DNSLookupError
from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import Codes, SynapseError
from synapse.http.client import SimpleHttpClient
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/thumbnail_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import SynapseError
from synapse.http.server import DirectServeJsonResource, set_cors_headers
Expand Down
11 changes: 7 additions & 4 deletions synapse/rest/media/v1/upload_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
# limitations under the License.

import logging
from typing import TYPE_CHECKING
from typing import IO, TYPE_CHECKING

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import Codes, SynapseError
from synapse.http.server import DirectServeJsonResource, respond_with_json
Expand Down Expand Up @@ -79,7 +79,9 @@ async def _async_render_POST(self, request: Request) -> None:
headers = request.requestHeaders

if headers.hasHeader(b"Content-Type"):
media_type = headers.getRawHeaders(b"Content-Type")[0].decode("ascii")
content_type_headers = headers.getRawHeaders(b"Content-Type")
assert content_type_headers # for mypy
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We know that the header will exist due to the above check, but mypy doesn't...

media_type = content_type_headers[0].decode("ascii")
else:
raise SynapseError(msg="Upload request missing 'Content-Type'", code=400)

Expand All @@ -88,8 +90,9 @@ async def _async_render_POST(self, request: Request) -> None:
# TODO(markjh): parse content-dispostion

try:
content = request.content # type: IO # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of this is undefined right now in Twisted (I think)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the type: ignore after the IO type definition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It overrides the definition previously calculated.

content_uri = await self.media_repo.create_content(
media_type, upload_name, request.content, content_length, requester.user
media_type, upload_name, content, content_length, requester.user
)
except SpamMediaException:
# For uploading of media we want to respond with a 400, instead of
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/synapse/client/new_user_consent.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import logging
from typing import TYPE_CHECKING

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import SynapseError
from synapse.handlers.sso import get_username_mapping_session_cookie_from_request
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/synapse/client/password_reset.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import logging
from typing import TYPE_CHECKING, Tuple

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import ThreepidValidationError
from synapse.config.emailconfig import ThreepidBehaviour
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/synapse/client/pick_username.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
import logging
from typing import TYPE_CHECKING, List

from twisted.web.http import Request
from twisted.web.resource import Resource
from twisted.web.server import Request

from synapse.api.errors import SynapseError
from synapse.handlers.sso import get_username_mapping_session_cookie_from_request
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/synapse/client/sso_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import logging
from typing import TYPE_CHECKING

from twisted.web.http import Request
from twisted.web.server import Request

from synapse.api.errors import SynapseError
from synapse.handlers.sso import get_username_mapping_session_cookie_from_request
Expand Down
2 changes: 2 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -189,5 +189,7 @@ commands=
[testenv:mypy]
deps =
{[base]deps}
# Type hints are broken with Twisted > 20.3.0, see https://github.com/matrix-org/synapse/issues/9513
twisted==20.3.0
extras = all,mypy
commands = mypy