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

[Question] whats mean about this todo(ThreadLocal) #2433

Open
1 task done
SunnyBoy-WYH opened this issue Jan 29, 2024 · 2 comments
Open
1 task done

[Question] whats mean about this todo(ThreadLocal) #2433

SunnyBoy-WYH opened this issue Jan 29, 2024 · 2 comments
Assignees
Labels
bug Something isn't working question Further information is requested
Milestone

Comments

@SunnyBoy-WYH
Copy link
Contributor

SunnyBoy-WYH commented Jan 29, 2024

Problem Type (问题类型)

rest-api (结果不合预期)

Before submit

  • 我已经确认现有的 IssuesFAQ 中没有相同 / 重复问题 (I have confirmed and searched that there are no similar problems in the historical issue and documents)

Environment (环境信息)

  • apache 1.2.0
  • windows
  • auth mode

Your Question (问题描述)

do you have any context info about this todo? @javeme (link the code below)

image
@dosubot dosubot bot added bug Something isn't working question Further information is requested labels Jan 29, 2024
Copy link

dosubot bot commented Jan 29, 2024

🤖

Hello @SunnyBoy-WYH!

I'm Dosu, a friendly bot here to assist you with your issues while we wait for a human maintainer. Whether it's solving bugs, answering your questions, or helping you become a contributor, I'm here to help! Let's work together to get your issue resolved.

Based on the information you've provided, it seems like the ThreadLocal variable CONTEXTS is used to store context information for each thread. This context information includes user data, which is set before a task is executed and is supposed to be reset after the task is done.

The potential security issue you've mentioned could arise if the resetContext() method is not called after a request is finished. In a multi-threaded environment like a web server, threads are often reused across different requests. If a ThreadLocal variable is not properly cleaned up after serving a request, the data stored in the variable might be exposed to and misused by other requests processed by the same thread later. This could lead to data leakage or incorrect behavior.

Here is the relevant code:

private static final ThreadLocal<String> CONTEXTS = new ThreadLocal<>();

protected static void setContext(String context) {
    CONTEXTS.set(context);
}

protected static void resetContext() {
    CONTEXTS.remove();
}

public static String getContext() {
    return CONTEXTS.get();
}

public static class ContextCallable<V> implements Callable<V> {

    private final Callable<V> callable;
    private final String context;

    public ContextCallable(Callable<V> callable) {
        E.checkNotNull(callable, "callable");
        this.context = getContext();
        this.callable = callable;
    }

    @Override
    public V call() throws Exception {
        setContext(this.context);
        try {
            return this.callable.call();
        } finally {
            resetContext();
        }
    }
}

In the call() method of ContextCallable, the context is set at the beginning of the method and reset at the end. This ensures that the context is properly cleaned up after the method is finished, preventing the potential security issue.

As for the TODO comment related to ThreadLocal, without the specific comment or the relevant code, it's hard to provide any specific information or context about it. If you could provide the exact comment or the relevant code, I might be able to give a more precise answer.

For more information, you can refer to the following sources:

I hope this helps! If you have any more questions or need further clarification, feel free to ask.

-Dosu

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@imbajin imbajin added this to the 1.3.0 milestone Jan 30, 2024
@javeme
Copy link
Contributor

javeme commented Feb 3, 2024

@SunnyBoy-WYH maybe we should unset in a rest filter, like LoadReleaseFilter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants