Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed StaticRoute working incorrectly with the same headerParam on mu… #1453

Merged
merged 1 commit into from
Apr 5, 2016
Merged

Fixed StaticRoute working incorrectly with the same headerParam on mu… #1453

merged 1 commit into from
Apr 5, 2016

Conversation

IrenejMarc
Copy link
Contributor

…ltiple methods in interface

When the same interface had multiple functions all using the same headerParam, the comparator for AnySatisfy was reused from previously-compared functions, resulting in completely incorrect results

Some details in the code below.

Example:

import vibe.d;

interface API
{
    Collection!FooCollection foos();
}

class APIImpl : API
{
    private FooCollection _foos;

    this()
    {
        _foos = new FooCollectionImpl;
    }

    Collection!FooCollection foos()
    {
        return Collection!FooCollection(_foos);
    }
}

enum fooHeader = headerParam("foo", "FooHeader");
interface FooCollection
{
    struct CollectionIndices
    {
        long _id;
    }


// foo is the second param here
    @fooHeader
        string getSomething(long asd, string foo);

// BOTH the second param and the third one will now be headerParams("<fieldname>", "FooHeader")
    @fooHeader
        string get(long _id, int bar, string foo);
}

class FooCollectionImpl : FooCollection
{

    string getSomething(long asd, string foo)
    {
        return foo;
    }

    string get(long _id, int bar, string foo)
    {
        return foo;
    }
}

shared static this()
{
    auto settings = new HTTPServerSettings;
    settings.port = 8080;
    settings.bindAddresses = ["::1", "127.0.0.1"];

    auto router = new URLRouter;
    router.registerRestInterface(new APIImpl);

    listenHTTP(settings, router);
}

@s-ludwig
Copy link
Member

I'm just wondering if using the manged name as an identifier is a good idea. For functions with D linkage (admittedly almost always the case), it should work, but for C++ linkage it might not for example. Safer might be to use fi.stringof.

@IrenejMarc
Copy link
Contributor Author

Seeing as this is not a GenCmp change, but just a change in the StaticRoute, that applies to only REST interfaces, is it even possible to have C++ linkage for those methods?

@s-ludwig
Copy link
Member

s-ludwig commented Apr 5, 2016

In theory it is, but admittedly highly unlikely. Ideally, GenCmp should be extended to support nested indices directly, I'll do that later when I get some time. But for now, the mangleof fix should do.

@s-ludwig s-ludwig merged commit ff33a81 into vibe-d:master Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants