Unverified Commit e52130b0 authored by mvdbeek's avatar mvdbeek
Browse files

Urlencode parameters in tagService.js 

Fixes
https://sentry.galaxyproject.org/share/issue/ff9f4891f34e4308b8183ef604970761/:
```
TypeError: tag_autocomplete_data() got an unexpected keyword argument 'd'
  File "uvicorn/protocols/http/h11_impl.py", line 366, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "uvicorn/middleware/proxy_headers.py", line 75, in __call__
    return await self.app(scope, receive, send)
  File "fastapi/applications.py", line 269, in __call__
    await super().__call__(scope, receive, send)
  File "starlette/applications.py", line 124, in __call__
    await self.middleware_stack(scope, receive, send)
  File "starlette/middleware/errors.py", line 184, in __call__
    raise exc
  File "starlette/middleware/errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "starlette/middleware/base.py", line 69, in __call__
    await response(scope, receive, send)
  File "starlette/responses.py", line 260, in __call__
    await wrap(partial(self.listen_for_disconnect, receive))
  File "anyio/_backends/_asyncio.py", line 662, in __aexit__
    raise exceptions[0]
  File "starlette/responses.py", line 256, in wrap
    await func()
  File "starlette/responses.py", line 245, in stream_response
    async for chunk in self.body_iterator:
  File "starlette/middleware/base.py", line 58, in body_stream
    raise app_exc
  File "starlette/middleware/base.py", line 36, in coro
    await self.app(scope, request.receive, send_stream.send)
  File "starlette_context/middleware/raw_middleware.py", line 96, in __call__
    await self.app(scope, receive, send_wrapper)
  File "starlette/middleware/base.py", line 69, in __call__
    await response(scope, receive, send)
  File "starlette/responses.py", line 260, in __call__
    await wrap(partial(self.listen_for_disconnect, receive))
  File "anyio/_backends/_asyncio.py", line 662, in __aexit__
    raise exceptions[0]
  File "starlette/responses.py", line 256, in wrap
    await func()
  File "starlette/responses.py", line 245, in stream_response
    async for chunk in self.body_iterator:
  File "starlette/middleware/base.py", line 58, in body_stream
    raise app_exc
  File "starlette/middleware/base.py", line 36, in coro
    await self.app(scope, request.receive, send_stream.send)
  File "starlette/middleware/base.py", line 69, in __call__
    await response(scope, receive, send)
  File "starlette/responses.py", line 260, in __call__
    await wrap(partial(self.listen_for_disconnect, receive))
  File "anyio/_backends/_asyncio.py", line 662, in __aexit__
    raise exceptions[0]
  File "starlette/responses.py", line 256, in wrap
    await func()
  File "starlette/responses.py", line 245, in stream_response
    async for chunk in self.body_iterator:
  File "starlette/middleware/base.py", line 58, in body_stream
    raise app_exc
  File "starlette/middleware/base.py", line 36, in coro
    await self.app(scope, request.receive, send_stream.send)
  File "starlette/exceptions.py", line 93, in __call__
    raise exc
  File "starlette/exceptions.py", line 82, in __call__
    await self.app(scope, receive, sender)
  File "fastapi/middleware/asyncexitstack.py", line 21, in __call__
    raise e
  File "fastapi/middleware/asyncexitstack.py", line 18, in __call__
    await self.app(scope, receive, send)
  File "starlette/routing.py", line 670, in __call__
    await route.handle(scope, receive, send)
  File "starlette/routing.py", line 418, in handle
    await self.app(scope, receive, send)
  File "a2wsgi/wsgi.py", line 140, in __call__
    return await responder(scope, receive, send)
  File "a2wsgi/wsgi.py", line 179, in __call__
    raise self.exc_info[0].with_traceback(
  File "galaxy/web/framework/middleware/error.py", line 165, in __call__
    app_iter = self.application(environ, sr_checker)
  File "/cvmfs/main.galaxyproject.org/venv/lib/python3.8/site-packages/paste/recursive.py", line 85, in __call__
    return self.application(environ, start_response)
  File "galaxy/web/framework/middleware/statsd.py", line 29, in __call__
    req = self.application(environ, start_response)
  File "/cvmfs/main.galaxyproject.org/venv/lib/python3.8/site-packages/paste/httpexceptions.py", line 640, in __call__
    return self.application(environ, start_response)
  File "galaxy/web/framework/base.py", line 159, in __call__
    return self.handle_request(environ, start_response)
  File "galaxy/web/framework/base.py", line 244, in handle_request
    body = method(trans, **kwargs)
  File "galaxy/web/framework/decorators.py", line 84, in decorator
    return func(self, trans, *args, **kwargs)
```

You can test this by editing a history tag in the history menu, by
writing `tag;d`, there should be no 500 error on the Galaxy logs.
parent 457a6275

client/src/components/Tags/tagService.js

+11 −10
Original line number Diff line number Diff line
@@ -57,10 +57,11 @@ export class TagService { 
        if (!tag.valid) {

            throw new Error("Invalid tag");

        }

        const url = `${getAppRoot()}tag/add_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&new_tag=${

            tag.text

        }`;

        const response = await axios.get(url);

        const url = `${getAppRoot()}tag/add_tag_async`;

        const config = {

            params: { item_id: id, item_class: itemClass, context: context, new_tag: tag.text },

        };

        const response = await axios.get(url, config);

        if (response.status !== 200) {

            throw new Error(`Unable to save tag: ${tag}`);

        }
@@ -75,10 +76,9 @@ export class TagService { 
    async delete(rawTag) {

        const { id, itemClass, context } = this;

        const tag = createTag(rawTag);

        const url = `${getAppRoot()}tag/remove_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&tag_name=${

            tag.text

        }`;

        const response = await axios.get(url);

        const url = `${getAppRoot()}tag/remove_tag_async`;

        const config = { params: { item_id: id, item_class: itemClass, context: context, tag_name: tag.text } };

        const response = await axios.get(url, config);

        if (response.status !== 200) {

            throw new Error(`Unable to delete tag: ${tag}`);

        }
@@ -92,8 +92,9 @@ export class TagService { 
     */

    async autocomplete(searchText) {

        const { id, itemClass } = this;

        const url = `${getAppRoot()}tag/tag_autocomplete_data?item_id=${id}&item_class=${itemClass}&q=${searchText}`;

        const response = await axios.get(url);

        const url = `${getAppRoot()}tag/tag_autocomplete_data`;

        const config = { params: { item_id: id, item_class: itemClass, q: searchText } };

        const response = await axios.get(url, config);

        if (response.status !== 200) {

            throw new Error(`Unable to retrieve autocomplete tags for search string: ${searchText}`);

        }

client/src/components/Tags/tagService.test.js

+17 −47
Original line number Diff line number Diff line
@@ -2,9 +2,6 @@ import axios from "axios"; 
import MockAdapter from "axios-mock-adapter";

import { TagService } from "./tagService";

import { createTag } from "./model";

//import { interval } from "rxjs";

//import { take, takeUntil } from "rxjs/operators";

// test response

import autocompleteResponse from "./testData/autocompleteResponse.txt";



describe("Tags/tagService.js", () => {
@@ -15,6 +12,12 @@ describe("Tags/tagService.js", () => { 
        context: "",

        debounceInterval: 50, // shorter value than default for unit tests

    };

    const testLabel = "fo0bar123";

    const expectedParams = {

        item_id: 123,

        item_class: "fooClass",

        context: "",

    };



    const svc = new TagService(svcParams);

    beforeEach(() => {
@@ -26,43 +29,43 @@ describe("Tags/tagService.js", () => { 
    });



    describe("save", () => {

        const testLabel = "fo0bar123";

        const testTag = createTag(testLabel);



        const { id, itemClass, context } = svcParams;

        const expectedSaveUrl = `/tag/add_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&new_tag=${testLabel}`;

        const expectedSaveUrl = `/tag/add_tag_async`;



        it("should save a string tag", async () => {

            const savedTag = await svc.save(testLabel);

            expect(savedTag.text).toBe(testLabel);

            expect(axiosMock.history.get[0].url).toBe(expectedSaveUrl);

            expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, new_tag: testLabel });

        });



        it("should save an object tag", async () => {

            const savedTag = await svc.save(testTag);

            expect(savedTag.text).toBe(testLabel);

            expect(axiosMock.history.get[0].url).toBe(expectedSaveUrl);

            expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, new_tag: testLabel });

        });



        // TODO: test error conditions

    });

    describe("delete", () => {

        const testLabel = "fo0bar123";

        const testTag = createTag(testLabel);



        const { id, itemClass, context } = svcParams;

        const expectedDeleteUrl = `/tag/remove_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&tag_name=${testLabel}`;

        const expectedDeleteUrl = `/tag/remove_tag_async`;



        it("should delete a text tag", async () => {

            const result = await svc.delete(testTag);

            expect(result).toBeTruthy();

            expect(axiosMock.history.get[0].url).toBe(expectedDeleteUrl);

            expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, tag_name: testLabel });

        });



        it("should delete an object tag", async () => {

            const result = await svc.delete(testTag);

            expect(result).toBeTruthy();

            expect(axiosMock.history.get[0].url).toBe(expectedDeleteUrl);

            expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, tag_name: testLabel });

        });



        // TODO: test error conditions
@@ -70,8 +73,7 @@ describe("Tags/tagService.js", () => { 


    describe("autocomplete", () => {

        const searchString = "foo";

        const { id, itemClass } = svcParams;

        const expectedSearchUrl = `/tag/tag_autocomplete_data?item_id=${id}&item_class=${itemClass}&q=${searchString}`;

        const expectedSearchUrl = `/tag/tag_autocomplete_data`;



        const checkAutocompleteResult = (result) => {

            expect(result).toBeTruthy();
@@ -83,46 +85,14 @@ describe("Tags/tagService.js", () => { 


        // straight ajax request, unused in practice, but it's easier to

        // test this call if we just expose it

        const { ...searchParams } = expectedParams;

        delete searchParams.context;



        it("ajax call should return tag objects", async () => {

            const result = await svc.autocomplete(searchString);

            expect(axiosMock.history.get[0].url).toBe(expectedSearchUrl);

            expect(axiosMock.history.get[0].params).toEqual({ ...searchParams, q: searchString });

            checkAutocompleteResult(result);

        });



        /*



        // hit the search input with multiple entries, only one ajax call

        // should result because of debouncing

        it("should debounce autocomplete search inputs", (done) => {

            // stub ajax request to return the success response if the

            // searchString is the expected input



            // take first emission, debouncing should guarantee the first

            // emission is the last thing we sent, should be searchString

            const timer$ = interval(100).pipe(take(1));

            const option$ = svc.autocompleteOptions.pipe(takeUntil(timer$));



            const nextHandler = jest.fn();



            option$.subscribe({

                next: nextHandler,

                error: (err) => console.warn("error", err),

                complete: () => {

                    expect(nextHandler).toHaveBeenCalledTimes(1);

                    expect(axiosMock.history.get.length).toBe(1);

                    done();

                },

            });



            // spam a bunch of key inputs followed by the correct input

            const spamCount = 2 + Math.floor(Math.random() * 10);

            for (let i = 0; i < spamCount; i++) {

                const spamVal = new String(Math.random());

                svc.autocompleteSearchText = spamVal;

            }

            svc.autocompleteSearchText = searchString;

        });



        */

    });

});